Доброго времени... У меня возникла необходимость в освоении ООП. Занимаюсь программирование на PHP - примерно год, тяжело переходить с процедурного кода на объектный. Эта моя первая, более-менее серьезная работа с ООП кодом. Если вам не трудно, прокомментируйте правильность. PHP: <?PHP class Voting{ private $filename; private $answer; private $data; private $vote = Array(); public function GetAnswer($var1, $var2){ $this->filename = $var1; //Приравниваем значения функции к аргументам класса $this->answer = $var2; $file = fopen($this->filename,'r'); $this->data = fread($file, filesize($this->filename)); $this->vote = explode('-',$this->data); fclose($file); } public function CheckIP($var3){ $counter = count($this->vote); if ($this->vote[$counter-1] !== $var3){ $this->vote[] = $var3; $this->WriteAns(); } } private function WriteAns(){ $file = fopen($this->filename,'w'); $this->vote[$this->answer]++; $this->data = fwrite($file, implode('-',$this->vote)); fclose($file); } public function GetPrint($var3){ return $this->vote[$var3]; } } ?> ЗАРАНЕЕ СПАСИБО!
alexander.pro во-первых комменты бы написал... во-вторых - по-моему много "лишних" свойств... Например в методе GetAnswer, зачем там $this->data ? Почему не использовать обычную локальную переменную? ООП - это же не значит, что все переменные нужно делать свойствами... А то так можно и для циклов (переменная-счетчик) - использовать свойства
и аргументы в методах... почему везде непонятное название $var1, $var2, $var3 ? Можно же дать более осмысленное название
PHP: <?php public function GetPrint($var3){ return $this->vote[$var3]; } тоже как-то странно... Знаешь, ты напиши лучше законченный класс с комментариями А то по такой "половинке" нифига не поймешь
ООП - это не внести все функции в класс и некоторые параметры сделать общими в виде свойств. Тут логика в том, чтобы вся логика была внутри класса. У меня стойкое ощущение, что куча логики вне этого класса, ну или не весь код. В любом случае сразу видно, что метод checkIP точно не нужен. Также как и остальные два Точнее, они точно не должны быть public. Ну и для начала, как уже сказал sylex, сделать вменяемые имена переменных и написать комментарии.
в моих поделках чаще всего паблик конструктор и метод show/process/run + какие-то установки типа setCount(5), setTemplate('folder/templare.htm');
Даже не знаю с чего начать... PHP: <?PHP class Voting{ private $filename; // Объявляю свойства класса (filename - имя файла answer - это вариант ответа) private $answer; private $vote = Array(); public function GetAnswer($filename, $answer){ $this->filename = $filename; // Присваиваем параметры функции свойствам класса $this->answer = $answer; $file = fopen($this->filename,'r'); // Открываю файл для получения статистики ответов $data = fread($file, filesize($this->filename)); // Читаю каждую строку файла (по правде говоря она там одна) $this->vote = explode('-',$data);// Разделяю строку ответов в массив fclose($file); // Закрываю файл } public function CheckIP($ip){ // Функция для определения IP $counter = count($this->vote); if ($this->vote[$counter-1] !== $ip){ // Если IP адрес голосующего не совпадает с тем, что уже в файле $this->vote[$counter-1] = $ip; // Переписываем IP пользователя $this->WriteAns(); //Записываем в файл } } private function WriteAns(){ $file = fopen($this->filename,'w'); $this->vote[$this->answer]++; $this->data = fwrite($file, implode('-',$this->vote)); fclose($file); } public function GetPrint($var3){ return $this->vote[$var3]; } } ?> PHP: <?php require ('./voting.php'); $vote=new Voting(); $vote->GetAnswer('./db.dat',2); $vote->CheckIP($_SERVER['REMOTE_ADDR']); echo $vote->GetPrint(2); ?> Содержание db.dat - "0-0-3-0-127.0.0.5"
Начинать надо с основ: прочитать про инкапсуляцию. В приведенном классе ее нет. 1. Инкапсулируем всю логику хранения данных о голосовавших в класс. Код, в котором используется этот класс не должен ни сном ни духом ведать о том, где и как все это хранится, как получается и как сохраняется. Почему? Да чтобы потом, когда данных станет много и возникнет необходимость использовать БД, мы делаем необходимые изменения в классе не затрагивая внешний код. Все как работало, так и работает: изменения только внутри одного класса. Отступление: тут вообще надо понять цель всего проектирования. А она не в том, чтобы написать код, который работает, а чтобы написать код, который легко поддерживать и в который легко вносить изменения. 2. Убираем всякие параметры типа "2". Что такое "2"? Нет, я конечно, раскопаю и разберусь, откуда эта два взялась и почему, но на это уйдет время, много времени. Почему бы не переименовать это дело в answerIndex или что это там… Зачем вообще внешний код должен знать о каких-то индексах или номерах или что там обозначают эти цифры? Внешнему коду надо что: возможность голосовать и получить данные голосования. Все. Внешнему коду не интересно что там и как у вас внутри класса. 3. Оставляем только нужные методы. Зачем нужен GetAnswer, который еще и не возвращает ничего, хотя начинается на get. Зачем нужен checkIP? В результате должно получится что-то вроде: PHP: <?php require ('./voting.php'); $vote=new Voting(); echo $vote->GetPrint(); ?> А все проверки, загрузки необходимых файлов и т.п. пусть делаются внутри класса, тогда, когда они нужны.
в дополнении примерчик - класс Пресс-центр - новости короче... PHP: <? class CPressCentre { /** * Возвращает данные о публикации * * @param int $iID - ID публикации * @param array $aParams - параметры * @return array */ public function GetOne($iID, $aParams = array()) { } /** * Возвращает список публикаций * * @param array $aParams - параметры * @param int $iLimit - количество на страницу * @return array */ public function GetList($aParams = array(), $iLimit = 10) { } /** * Удаляет выбранные публикации * * @param array $aIDs - массив ID публикаций * @return bool */ public function RemoveSelected($aIDs) { } /** * Добавляет публикацию * * @param array $aFields - поля * @return bool */ public function Add($aFields) { if (!$this->CheckFields) return false; } /** * Изменяет данные публикации * * @param int $iID - ID публикации * @param array $aFields - поля * @return bool */ public function Update($iID, $aFields) { if (!$this->CheckFields) return false; } /** * Проверка полей * Возвращает true если проверка прошла успешно * @param array $aFields - поля * @return bool */ private function CheckFields($aFields) { } } ?>