Pomoc - Szukaj - Użytkownicy - Kalendarz
Pełna wersja: [PHP]Sprawdzenie klasy
Forum PHP.pl > Forum > Przedszkole
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.
  1. <?php
  2.  
  3. print_r($_POST);
  4.  
  5. class AirConditioning {
  6.  
  7. const HOME = 20;
  8. const HOME_ROOF = 0.2;
  9. const COMPANY = 10;
  10. const COMPANY_PERCENT = 0.1;
  11. const COMPANY_ROOF = 0.1;
  12. const PEOPLE = 0.1;
  13.  
  14. private $people;
  15. //public $window;
  16. private $roof;
  17.  
  18. public function __construct($room, $height, $area, $people, $window, $roof) {
  19. $this->room = $room;
  20. $this->height = $height;
  21. $this->area = $area;
  22. if(!empty($window)) $this->window = $window;
  23. if(!empty($people)) $this->people = $people;
  24. if(!empty($roof)) $this->roof = $roof;
  25. }
  26.  
  27. public function check() {
  28. if($this->room == 0) {
  29. return $this->home($this->height, $this->area);
  30. }
  31. if($this->room == 1) {
  32. return $this->company($this->height, $this->area);
  33. }
  34. }
  35.  
  36. public function home($height, $area) {
  37. $count = $height*$area/self::HOME;
  38.  
  39. if($this->roof == 1) {
  40. $roof = self::HOME_ROOF;
  41. }
  42.  
  43. if($this->people > 0) {
  44. $people = $this->people($this->people);
  45. }
  46.  
  47. if(isset($roof) AND $roof > 0) {
  48. $count = $count+$count*$roof;
  49. }
  50.  
  51. if(isset($people) AND $people > 0) {
  52. $count = $count+$people;
  53. }
  54.  
  55. return $count;
  56. }
  57.  
  58. public function company($height, $area) {
  59. $count = $height*$area;
  60. $count = $count/self::COMPANY;
  61. $add = self::COMPANY_PERCENT;
  62.  
  63. if($this->roof == 1) {
  64. $add += self::COMPANY_ROOF;
  65. }
  66.  
  67. if($this->people > 0) {
  68. $people = $this->people($this->people);
  69. }
  70.  
  71. if(isset($roof) AND $roof > 0) {
  72. $count = $count+$count*$roof;
  73. }
  74.  
  75. if(isset($people) AND $people > 0) {
  76. $count = $count+$people;
  77. }
  78.  
  79. return $count;
  80. }
  81.  
  82. public function people($people) {
  83. $people = $people*self::PEOPLE;
  84. return $people;
  85. }
  86.  
  87. }
  88.  
  89. ?>




rocktech.pl
Witam.

Trochę szczegółów, komentarze, przykłady użycia poprosimy.

Na ten moment PHP_CodeSniffer smile.gif

  1. phpcs AirConditioning.class.php
  2.  
  3. FILE: AirConditioning.class.php
  4. --------------------------------------------------------------------------------
  5. FOUND 25 ERROR(S) AND 3 WARNING(S) AFFECTING 22 LINE(S)
  6. --------------------------------------------------------------------------------
  7. 2 | ERROR | Missing file doc comment
  8. 5 | ERROR | Missing class doc comment
  9. 5 | ERROR | Opening brace of a class must be on the line after the
  10. | | definition
  11. 14 | ERROR | Private member variable "people" must be prefixed with an
  12. | | underscore
  13. 16 | ERROR | Private member variable "roof" must be prefixed with an
  14. | | underscore
  15. 18 | ERROR | You must use "/**" style comments for a function comment
  16. 18 | ERROR | Opening brace should be on a new line
  17. 22 | WARNING | Inline control structures are discouraged
  18. 23 | WARNING | Inline control structures are discouraged
  19. 24 | WARNING | Inline control structures are discouraged
  20. 27 | ERROR | Missing function doc comment
  21. 27 | ERROR | Opening brace should be on a new line
  22. 28 | ERROR | Expected "if (...) {\n"; found "if(...) {\n"
  23. 31 | ERROR | Expected "if (...) {\n"; found "if(...) {\n"
  24. 36 | ERROR | Missing function doc comment
  25. 36 | ERROR | Opening brace should be on a new line
  26. 39 | ERROR | Expected "if (...) {\n"; found "if(...) {\n"
  27. 43 | ERROR | Expected "if (...) {\n"; found "if(...) {\n"
  28. 47 | ERROR | Expected "if (...) {\n"; found "if(...) {\n"
  29. 51 | ERROR | Expected "if (...) {\n"; found "if(...) {\n"
  30. 58 | ERROR | Missing function doc comment
  31. 58 | ERROR | Opening brace should be on a new line
  32. 63 | ERROR | Expected "if (...) {\n"; found "if(...) {\n"
  33. 67 | ERROR | Expected "if (...) {\n"; found "if(...) {\n"
  34. 71 | ERROR | Expected "if (...) {\n"; found "if(...) {\n"
  35. 75 | ERROR | Expected "if (...) {\n"; found "if(...) {\n"
  36. 82 | ERROR | Missing function doc comment
  37. 82 | ERROR | Opening brace should be on a new line
  38. --------------------------------------------------------------------------------
minolone
~rocktech.pl, myśle że "doc comment" sobie odpuścimy
proszę o sprawdzenie kodu czy jest to napisane w dość dobry sposób
pozdrawiam

  1. <?php
  2.  
  3. class AirConditioning
  4. {
  5.  
  6. const HOME = 20;
  7. const HOME_ROOF = 0.2;
  8. const COMPANY = 10;
  9. const COMPANY_PERCENT = 0.1;
  10. const COMPANY_ROOF = 0.1;
  11. const PEOPLE = 0.1;
  12.  
  13. public $people;
  14. public $roof;
  15.  
  16. public function __construct ($room, $height, $area, $people, $window, $roof)
  17. {
  18. $this->room = $room;
  19. $this->height = $height;
  20. $this->area = $area;
  21.  
  22. if (! empty($window)) {
  23. $this->window = $window;
  24. }
  25.  
  26. if (! empty($people)) {
  27. $this->people = $people;
  28. }
  29.  
  30. if (! empty($roof)) {
  31. $this->roof = $roof;
  32. }
  33. }
  34.  
  35. public function check ()
  36. {
  37. if ($this->room == 0) {
  38. return $this->home($this->height, $this->area);
  39. }
  40.  
  41. if ($this->room == 1) {
  42. return $this->company($this->height, $this->area);
  43. }
  44. }
  45.  
  46. public function home ($height, $area)
  47. {
  48. $count = $height * $area / self::HOME;
  49.  
  50. if ($this->roof == 1) {
  51. $roof = self::HOME_ROOF;
  52. }
  53.  
  54. if ($this->people > 0) {
  55. $people = $this->people($this->people);
  56. }
  57.  
  58. if (isset($roof) and $roof > 0) {
  59. $count = $count + $count * $roof;
  60. }
  61.  
  62. if (isset($people) and $people > 0) {
  63. $count = $count + $people;
  64. }
  65.  
  66. return $count;
  67. }
  68.  
  69. public function company ($height, $area)
  70. {
  71. $count = $height * $area;
  72. $count = $count / self::COMPANY;
  73. $add = self::COMPANY_PERCENT;
  74.  
  75. if ($this->roof == 1) {
  76. $add += self::COMPANY_ROOF;
  77. }
  78.  
  79. if ($this->people > 0) {
  80. $people = $this->people($this->people);
  81. }
  82.  
  83. if (isset($roof) and $roof > 0) {
  84. $count = $count + $count * $roof;
  85. }
  86.  
  87. if (isset($people) and $people > 0) {
  88. $count = $count + $people;
  89. }
  90.  
  91. return $count;
  92. }
  93.  
  94. public function people ($people)
  95. {
  96. $people = $people * self::PEOPLE;
  97. return $people;
  98. }
  99.  
  100. }
  101.  
  102. ?>
rocktech.pl
Hej właśnie komentarz chciałbym zobaczyć. Na pierwszy rzut oka domyślam się, że klasa ma klimatyzować ...
Znacznie łatwiej byłoby się połapać potencjalnemu oceniającemu gdybyś zamieścił opis, przykłady użycia oraz komentarze, które odzwierciedlają tok myślenia autora i same w sobie mogą stanowić przedmiot oceny.

Pozdrawiam, jak ty się przyłożysz to inni też smile.gif
minolone
Proszę o sprawdzenie poprawności kodu oraz ew. poprawki,
Napewno jakieś błędy się znajdą ale proszę o wyrozumiałość.
Ogólnie wszystko działa tak jak powinno.

  1. <?php
  2. /**
  3.  * AirConditioning
  4.  * Klasa odpowiedzialna za wyliczanie mocy klimatyzatora
  5.  */
  6. class AirConditioning {
  7. const HOME = 20; //Dzielenie wyniku m3 dla pomieszczeń mieszkalnych
  8. const HOME_ROOF = 0.2; //Dodatek 20% dla okien dachowych, mieszkalnych
  9. const COMPANY = 10; //Dzielenie wyniku m3 dla pomieszczeń handlowych
  10. const COMPANY_PERCENT = 0.1; //Dodatek 10% dla pomieszczeń handlowych
  11. const COMPANY_ROOF = 0.1; //Dodatek 10% dla okien dachowych, handlowych
  12. const PEOPLE = 0.1; //Dodatek 0.1 kW za każdego człowieka
  13. const COMPANY_SOUTH = 0.05; //Dodatek 5% za okna skierowane na południe, pomieszczenia handlowe
  14. const COMPANY_WEST = 0.05; //Dodatek 5% za okna skierowane na zachód, pomieszczenia handlowe
  15. const HOME_SOUTH = 0.1; //Dodatek 10% za okna skierowane na południe, pomieszczenia mieszkalne
  16. const HOME_WEST = 0.1; //Dodatek 10% za okna skierowane na zachód, pomieszczenia mieszkalne
  17.  
  18. public $people;
  19. public $roof;
  20. public $south;
  21. public $west;
  22.  
  23. /**
  24.   * AirConditioning::__construct()
  25.   * Pobranie danych z formularza
  26.   * @param mixed $room informacje o pomieszczeniu
  27.   * @param mixed $height wysokość pomieszzenia
  28.   * @param mixed $area powierzchnia pomieszczenia
  29.   * @param mixed $people informacje o osobach w pomieszczeniu
  30.   * @param mixed $roof okna dachowe
  31.   * @param mixed $south kierunek okien, południe
  32.   * @param mixed $west kierunek okien, zachód
  33.   */
  34. public function __construct($room, $height, $area, $people, $roof, $south, $west)
  35. {
  36. $this->room = $room;
  37. $this->height = $height;
  38. $this->area = $area;
  39.  
  40. if (!empty($north)) {
  41. $this->north = $north;
  42. }
  43.  
  44. if (!empty($south)) {
  45. $this->south = $south;
  46. }
  47.  
  48. if (!empty($east)) {
  49. $this->east = $east;
  50. }
  51.  
  52. if (!empty($west)) {
  53. $this->west = $west;
  54. }
  55.  
  56. if (!empty($people)) {
  57. $this->people = $people;
  58. }
  59.  
  60. if (!empty($roof)) {
  61. $this->roof = $roof;
  62. }
  63. }
  64.  
  65. /**
  66.   * AirConditioning::check()
  67.   * Pobranie danych o pomieszczeniu
  68.   */
  69. public function check()
  70. {
  71. if ($this->room == 1) {
  72. return $this->home($this->height, $this->area);
  73. }
  74.  
  75. if ($this->room == 2) {
  76. return $this->company($this->height, $this->area);
  77. }
  78. }
  79.  
  80. /**
  81.   * AirConditioning::home()
  82.   * Obliczanie mocy klimatyzatora dla pomieszczeń mieszkalnych
  83.   */
  84. public function home($height, $area)
  85. {
  86. $count = $height * $area / self::HOME;
  87. $add = '';
  88.  
  89. if ($this->roof == 1) {
  90. $add += self::HOME_ROOF;
  91. }
  92.  
  93. if ($this->people > 0) {
  94. $people = $this->people($this->people);
  95. }
  96.  
  97. if ($this->west == 1) {
  98. $add += self::HOME_WEST;
  99. }
  100.  
  101. if ($this->south == 1) {
  102. $add += self::HOME_SOUTH;
  103. }
  104.  
  105. $count = $count + $count * $add;
  106.  
  107. if (isset($people) and $people > 0) {
  108. $count = $count + $people;
  109. }
  110.  
  111. return $count;
  112. }
  113.  
  114. /**
  115.   * AirConditioning::company()
  116.   * Obliczanie mocy klimatyzatora dla pomieszczeń handlowych
  117.   */
  118. public function company($height, $area)
  119. {
  120. $count = $height * $area;
  121. $count = $count / self::COMPANY;
  122. $add = self::COMPANY_PERCENT;
  123.  
  124. if ($this->west == 1) {
  125. $add += self::COMPANY_WEST;
  126. }
  127.  
  128. if ($this->south == 1) {
  129. $add += self::COMPANY_SOUTH;
  130. }
  131.  
  132. if ($this->roof == 1) {
  133. $add += self::COMPANY_ROOF;
  134. }
  135.  
  136. if ($this->people > 0) {
  137. $people = $this->people($this->people);
  138. }
  139.  
  140. $count = $count + $count * $add;
  141.  
  142. if (isset($people) and $people > 0) {
  143. $count = $count + $people;
  144. }
  145.  
  146. return $count;
  147. }
  148.  
  149. /**
  150.   * AirConditioning::people()
  151.   * Wylicza moc dla każdej osoby w pomieszczeniu
  152.   */
  153. public function people($people)
  154. {
  155. $people = $this->people * self::PEOPLE;
  156.  
  157. return $people;
  158. }
  159.  
  160. }
  161.  
  162. ?>

Przykład użycia
  1. <?php
  2. $ob = new AirConditioning($room, $height, $area, $people, $roof, $north, $south, $east, $west);
  3. echo 'Wyliczone zapotrzebowanie na moc klimatyzatora wynosi '. $ob->check(). ' kW.';
  4. ?>

wookieb
Nie pokazuj raportu ze Sniffera bo każdy ma inne standardy kodowania. Mnie np nie odpowiada ani Zend-owy ani Symfony2 więc jak?
minolone
~wookieb, ogólnie chodzi mi o sprawdzenie czy jest to jakoś tak w miare napisane, oraz ew. co powinienem zmienić, poprawić przy pisaniu.
Ogólnie to co chciałem uzyskać jest zrobione, czyli wylicza mi moc klimatyzatora tak jak powinno.
rocktech.pl
Proponuję PHPUnit.
minolone
~rocktech.pl, jakoś nie możemy się dogadać, ja nie potrzebuje robić jakiś testów na tej klasie, tylko dowiedzieć się o tym jak jest napisana i co ew zmienić, poprawić. Jest to do użytku wewnętrznego, testami bede się zajmował w innym czasie.
rocktech.pl
Hej moja wina przeczytałem
Cytat
czy wylicza mi moć klimatyzatora tak jak powinno.


Co do klasy ta metoda i własność i stała o tej samej nazwie są trochę mylące.

  1.  
  2. public function people($people) {
  3.  
  4. $people = $people*self::PEOPLE;
  5.  
  6. return $people;
  7.  
  8. }

  1. if ($this->people > 0) {
  2.  
  3. $people = $this->people($this->people);
  4.  
  5. }


Dalej co do parametrów raczej boolean zamiast mixed?

  1. * @param mixed $south kierunek okien, południe
  2.  
  3. * @param mixed $west kierunek okien, zachód
  4. .....
  5.  
  6. //i to sparwdzanie w konstruktorze bez sensu od razu przypisz wartość
  7. if (!empty($south)) {
  8.  
  9. $this->south = (bool) $south;
  10.  
  11. }


Potem w metodzie home
  1. ($this->south) AND $add += self::HOME_SOUTH;
  2. //a nie
  3. if($this->south == 1) {
  4. $add += self::HOME_SOUTH;
  5. }



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:
  1. public function home(...) {
  2. ...
  3.  
  4. if (...) {
  5. $people = ...;
  6. }
  7.  
  8. echo isset($people) ? $people : 'n/a';
  9. }
Nigdy nie powinieneś tego robić. Zdefiniuj wcześniej tą zmienną:
  1. public function home(...) {
  2. $people = null;
  3. ...
  4.  
  5. if (...) {
  6. $people = ...;
  7. }
  8.  
  9. echo !is_null($people) ? $people : 'n/a';
  10. }
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.
minolone
~Crozin, super że zainteresowałeś się moją klasą i dostałem tak spore informacje.
Ad.4 - zapomniałem usunąć z kodu,
Ad.2 - Raczej zmieniać się nie będą, na obecną chwilę oczywiście, może w przyszłości gdy to rozbuduje, narazie chciałem coś napisać i dostać wskazówki co robić a czego nie.
Ad.5 - Dokumentacja została wygenerowana i po prostu nie pozmieniałem, mój błąd.
Ad.7 - Wartości mogą być tylko 1 lub 2, pobierane z checkbox-a


Nie zostaje mi nic innego jak przepisać z twoimi wskazówkami.
Pozdrawiam, Dzięki
Crozin
Cytat
Ad.2 - Raczej zmieniać się nie będą, na obecną chwilę oczywiście, może w przyszłości gdy to rozbuduje, narazie chciałem coś napisać i dostać wskazówki co robić a czego nie.
Nawet nie chodzi o to czy się zmienią teraz czy później. Takie rzeczy z natury nie są stałymi, w dodatku nie odnosisz tutaj żadnych korzyści z ich użycia. Pewnego dnia te wartości zmienią się i będzie problem. Problem wynikający z błędnego zaplanowania. Coś jak w przypadku VAT-u, gdzie w wielu aplikacjach ludzie myśleli, że 22% to stała. wink.gif
Zamień to więc na normalną właściowść, a najlepiej na osobny obiekt (zbiór tych wartości to jedna właściwość, ale sama w sobie złożna z kilku innych; przy okazji punkt #10 będzie wtedy śmiesznie prosty do rozwiązania, a interfejs konstruktra znacznie się prości).
Cytat
Ad.7 - Wartości mogą być tylko 1 lub 2, pobierane z checkbox-a
  1. $ac = new AirConditioning(6, ...);
  2. $result = $ac->check();
Skoro mogą być tylko dwie to taki kod powinien wyrzucić wyjątek, a tego nie zrobi.
To jest wersja lo-fi głównej zawartości. Aby zobaczyć pełną wersję z większą zawartością, obrazkami i formatowaniem proszę kliknij tutaj.
Invision Power Board © 2001-2025 Invision Power Services, Inc.