[PHP]Sprawdzenie klasy Opcje

[minolone]

Witam.
Prosze was o sprawdzenie klasy oraz wyrzucenie mi wszystkich błędów co jest nie tak jak być powinno.
Jest nie skończona ale wszytsko działa tak jak chciałem.

[PHP] pobierz, plaintext 
<?php
 
    print_r($_POST);
 
    class AirConditioning {
 
        const HOME = 20; 
        const HOME_ROOF = 0.2;
        const COMPANY = 10;
        const COMPANY_PERCENT = 0.1;
        const COMPANY_ROOF = 0.1;
        const PEOPLE = 0.1;
 
        private $people;
        //public $window;
        private $roof;
 
        public function __construct($room, $height, $area, $people, $window, $roof) {
            $this->room = $room;
            $this->height = $height;
            $this->area = $area;
            if(!empty($window)) $this->window = $window;
            if(!empty($people)) $this->people = $people;
            if(!empty($roof)) $this->roof = $roof;
        }
 
        public function check() {
            if($this->room == 0) {
                return $this->home($this->height, $this->area);
            }
            if($this->room == 1) {
                return $this->company($this->height, $this->area);
            }
        }
 
        public function home($height, $area) {
            $count = $height*$area/self::HOME;
 
            if($this->roof == 1) {
                $roof = self::HOME_ROOF;
            }
 
            if($this->people > 0) {
                $people = $this->people($this->people);   
            }
 
            if(isset($roof) AND $roof > 0) {
                $count = $count+$count*$roof;
            }
 
            if(isset($people) AND $people > 0) {
                $count = $count+$people;
            }
 
            return $count;
        }
 
        public function company($height, $area) {
            $count = $height*$area;
            $count = $count/self::COMPANY;
            $add = self::COMPANY_PERCENT;
 
            if($this->roof == 1) {
                $add += self::COMPANY_ROOF;
            }
 
            if($this->people > 0) {
                $people = $this->people($this->people);   
            }
 
            if(isset($roof) AND $roof > 0) {
                $count = $count+$count*$roof;
            }
 
            if(isset($people) AND $people > 0) {
                $count = $count+$people;
            }
 
            return $count;
        }
 
        public function people($people) {
            $people = $people*self::PEOPLE;
            return $people;
        }
 
    } 
 
?>
[PHP] pobierz, plaintext 

[Crozin]

1. Powinieneś starać się by nazwa klasy była rzeczownikiem, a nazwy metod czasownikami - kod staje się wtedy znacznie bardziej "naturalny". W chwili obecnej Twój kod (przykładowo: $ac->people(), "klimatyzacja -> ludzie") nie ma żadnego sensu (z puntu widzenia normalnego czytania).
2. Jesteś pewien, że te wszystkie dodatki powinny być stałymi? Jestem niemal pewien, że cześć z tych wartości mogłaby się w przyszłości zmienić.
3. Nie jestem pewien co do sposobu użycia tego kodu, ale wydaje się, że jedynie konstruktor i metoda check() powinny być publiczne.
4. Używasz niezadeklarowanych właściwości obiektu (patrz: north, east).
5. Powinieneś dodać jakieś sprawdzanie poprawności przekazywanych argumentów, np. w konstruktorze (szczególnie gdy w dokumentacji podajesz mixed jako typ argumentu).
6. Nigdy nie używaj jakiś "magicznych" numerków w kodzie (patrz: if ($this->room == 1)). Zamiast nich użyj stałych, znacznie poprawia to cztelność kodu i daje możliwość jego wygodnej dokumentacji.
7. Właściwość room może przyjąć dowolną wartość, a tymczasem Ty zakładasz jedynie pojawienie się wartości 1 lub 2. Powinieneś wyrzucić jakiś wyjątek, gdy próbuje się podać inną.
8. Mimo iż PHP pozwala na zadeklarowanie i zdefiniowanie zmiennej w bloku przykładowo IF-a i późniejsze użycie tej zmiennej poza tym blokiem:

[PHP] pobierz, plaintext 
public function home(...) {
   ...
 
   if (...) {
      $people = ...;
   }
 
   echo isset($people) ? $people : 'n/a';
}
[PHP] pobierz, plaintext 

Nigdy nie powinieneś tego robić. Zdefiniuj wcześniej tą zmienną:

[PHP] pobierz, plaintext 
public function home(...) {
   $people = null;
   ...
 
   if (...) {
      $people = ...;
   }
 
   echo !is_null($people) ? $people : 'n/a';
}
[PHP] pobierz, plaintext 

9. Właściwości north, south, west i east powinny być chyba typu boolean?
10. Metody home() i company() (nieodpowiednie nazwy swoją drogą) wyglądają mi na dokładnie takie same z tą różnicą, że jedna operuje na stałych HOEM_*, a druga na COMPANY_*, racja? Bezproblemu zrobiłbyś z tego jedną metodę z trzecim argumentem przyjmującym obiekt/tablicę (typu wyliczeniowego w PHP niestety nie ma) z tymi stałymi.

Generalnie - do całościowego przepisania.