Witajcie
Napisałem sobie klasę do sprawdzania statusu serwera (włączony/wyłączony). Możecie go ocenić? (pod względem czy jest napisany poprawnie).

[PHP] pobierz, plaintext 
<?php
 
Class Status
{
	public function Get($ip, $port)
	{
		global $msg;
		$this->ip   = $ip;
		$this->port = $port;
		$this->msg  = $msg;
		$connection = fsockopen($this->ip, $this->port, $errorNo, $errorStr, 3);
		if($connection)
		{
			$this->msg = "Server is UP";
		}
		else
		{
			$this->msg = "Server is DOWN";
		}
	}
	public function Display()
	{
		echo $this->msg;
	}
}
 
$pub = new Status();
$pub->Get("25.135.219.119","25565");
$pub->Display();
 
?>
[PHP] pobierz, plaintext 

1. Daj @ przed fsockopen bo jak sie nie uda polączenie to przewie Ci skypt
2. Zadeklaruj zmienne ip, port, msg w klasie przed ich użyciem.
3. Zamiast Get bardziej by pasowała nazwa Set
4. Co to za zwyczaj nazywania metod z wielkich liter?
5. Co to za instrukcja global $msg bo nie rozumiem inicjatywy

1. Zrobione
2. Czemu? W PHP na wiki jest napisane "Zmienne w PHP nie wymagają deklaracji"
3. Zrobione
4. A nie wiem, z tutoriali pewnie
5. Gdy nie dam global $msg to będę miał Notice: Undefined variable: msg in bla bla

2. To że nie wymagają nie znaczy, że nie warto tego robić bo później klasa się rozrośnie jak to zwykle bywa i będziesz szukał w której metodzie została zadeklarowana taka zmienna i co robi a jak zadeklarujesz to wszystkie będziesz miał w jednym miejscu
5. Ale po co Ci ona skoro i tak ją nadpisujesz później:
if($connection)
{
$this->msg = "Server is UP";
}
else
{
$this->msg = "Server is DOWN";
}

Wywal: global $msg; i $this->msg = $msg;

Na początkowym etapie nauki radzę na siłę unikać zmiennych globalnych.

2. Zrobione, przemyślałem sobie i doszedłem do wniosku, że masz rację. Dzięki
5. Usunąłem global + $this->msg = $msg i działa pięknie, dzięki jeszcze raz!

Ogólnie rzecz biorąc, napisałem sobie drugi, taki sam skrypt. Według was - który lepszy?
Robiłem benchmark i różnica pomiędzy skryptami wynosi 0,000061.

Wersja 1:

[PHP] pobierz, plaintext 
Class Status
{
	public function Set($ip, $port = 80)
	{
		$this->ip   = $ip;
		$this->port = $port;
		$connection = @fsockopen($this->ip, $this->port, $errorNo, $errorStr, 3);
		if($connection)
		{
			$this->msg = "Server is UP";
		}
		else
		{
			$this->msg = "Server is DOWN";
		}
		fclose($connection);
	}
	public function Display()
	{
		echo $this->msg;
	}
	public static function getmicrotime()
	{
		return array_sum(explode(' ', microtime()));
	}
}
[PHP] pobierz, plaintext 

Wersja 2:

[PHP] pobierz, plaintext 
Class Status
{
	public function __construct($ip, $port = 80)
	{
		$this->Set($ip, $port);
		$this->Connect();
	}
	public function Set($ip, $port)
	{
		$this->ip   = $ip;
		$this->port = $port;
	}
	public function Connect()
	{
		$con = @fsockopen($this->ip, $this->port, $errorNo, $errorStr, 3);
		if($con)
		{
			$this->msg = "+";
		}
		else
		{
			$this->msg = "-";
		}
	}
	public function Display()
	{
		echo $this->msg;
	}
	public static function getmicrotime()
	{
		return array_sum(explode(' ', microtime()));
	}
}
[PHP] pobierz, plaintext 

Jakie macie uwagi?

Witam.

1. Nazwa klasy jest zbyt ogólna (czy używając tej klasy zobaczę obciążenie procesora na serwerze?)
2. Metoda Set() (ale co ..), jak już to użyj setterów setIP setPort
3. Connect() owszem można i tak ale niech Connect() wykonuje tylko operacje łączenia
4. Własność msg ma się nijak + - i, wyświetlaj (plus,OK,COOL,NICE) lub (minus,FAIL,BAD,WRONG) w zależności do statusu tu jest to wręcz zakazane

[PHP] pobierz, plaintext 
<?php
 
class ServerConnectionStatus
{
 
    public $ip = null;
    public $port = null;
    public $active = false;
 
    public function __construct($ip, $port = 80)
    {
 
        $this->ip = $ip;
        $this->port = $port;
    }
 
    public function check()
    {
        $con = @fsockopen($this->ip, $this->port, $errorNo, $errorStr, 3);
        if ( is_resource($con) ) {
            $this->active = true;
        } else {
            $this->active = false;
            throw new RuntimeException($errorStr);
        }
    }
 
    public function isActive()
    {
        return $this->active;
    }
 
}
[PHP] pobierz, plaintext 

[PHP] pobierz, plaintext 
$serverCStatus = new ServerConnectionStatus('localhost', 80);
try {
    $serverCStatus->check();
    echo $serverCStatus->isActive() ? '+' : '-';
} catch ( Exception $e ) {
    echo $e->getMessage();
}
[PHP] pobierz, plaintext