Warning: Cannot use a scalar value as an array in /home/admin/public_html/forum/include/fm.class.php on line 757

Warning: Invalid argument supplied for foreach() in /home/admin/public_html/forum/include/fm.class.php on line 770

Warning: Invalid argument supplied for foreach() in /home/admin/public_html/forum/topic.php on line 737
Форумы портала PHP.SU :: Документирование кода

 PHP.SU

 Программирование на PHP, MySQL и другие веб-технологии
PHP.SU Портал     На главную страницу форума Главная     Помощь Помощь     Поиск Поиск     Поиск Яндекс Поиск Яндекс     Вакансии  Пользователи Пользователи


 Страниц (1): [1]   

> Описание: Правильность документирование кода
Поиск в теме | Версия для печати
lodar
Отправлено: 16 Октября, 2013 - 02:21:02
Post Id


Новичок


Покинул форум
Сообщений всего: 20
Дата рег-ции: Авг. 2009  


Помог: 0 раз(а)
Возник вопрос по документированию кода. Например есть некий класс, в нем метод который выкидывает исключение. Этот метод вызывается конструктором. Нужно ли документировать исключение в пояснениях конструктора?

PHP:
скопировать код в буфер обмена
  1.  
  2.     /**
  3.      * Инициализирует настройки, устанавливает соединение
  4.      * Базовый конструктор, инициализирует настройки, формирует строку dsn и устанавливает соединение
  5.      * @param DBSettings $settings данные для подключения к бд {@link http://php.net}
  6.      * @throws WDBException
  7.      * @since 0.0.1
  8.      */
  9.     public function __construct(DBSettings $settings) {
  10.         $this->_settings = $settings;
  11.         $this->_dsn = "mysql:host=" . $settings->getProperty(DBSettings::DBHOST) . ";dbname=" . $settings->getProperty(DBSettings::DBNAME);
  12.         $this->setPDO();
  13.     }
  14.  


Метод, вызывающий исключение

PHP:
скопировать код в буфер обмена
  1.  
  2.     /**
  3.      * Устанавливает соединение с бд, в случае ошибок формирует исключение PDOException
  4.      * Указывает атрибуты по умолчанию для установленного соединения
  5.      * @throws WDBException
  6.      * @since 0.0.1
  7.      */
  8.     private function setPDO() {
  9.         $user = $this->getSettings()->getProperty(DBSettings::DBUSER);
  10.         $pass = $this->getSettings()->getProperty(DBSettings::DBPASS);
  11.  
  12.         try {
  13.             $this->_pdo = new PDO($this->getDsn(), $user, $pass);
  14.             $this->setDefaultPDOAttributes();
  15.         } catch (PDOException $e) {
  16.             throw new WDBException($e);
  17.         }
  18.     }
  19.  
 
 Top
tato
Отправлено: 16 Октября, 2013 - 04:32:38
Post Id



Посетитель


Покинул форум
Сообщений всего: 468
Дата рег-ции: Сент. 2011  
Откуда: Владивосток


Помог: 8 раз(а)
Если метод кидает исключение тогда да, в Вашем случае оно не кидается в конструкторе.


-----
просто ?: сложно
 
 Top
Мелкий Супермодератор
Отправлено: 16 Октября, 2013 - 09:55:19
Post Id



Активный участник


Покинул форум
Сообщений всего: 11926
Дата рег-ции: Июль 2009  
Откуда: Россия, Санкт-Петербург


Помог: 618 раз(а)
Мне кажется логичным указать это в комментарии ко всему классу. Что от использования класса можно ожидать исключение с таким-то именем.
Здесь надо решить, что важнее - документация кода или усилия, на это затрачивающиеся. Если комментарий надо исправить в двух местах - когда-нибудь это будет не сделано. А противоречивая документация хуже её отсутствия.


Хочу заметить по непосредственно коду - у вас очень загадочным образом размазаны настройки подключения. Почему dsn - в конструкторе, а логин и пароль - в подключении?
И раз класс явно называется DBSettings, то почему getDSN не в нём?
Почему сделано getProperty я понимаю, удобство обращения к массиву. Но почему бы для явным образом названного класса не повесить публичные getUsername, getPassword? Они лаконичнее, а изнутри могут проксировать к getProperty.

Ну и кидать исключения в конструкторе - не самая лучшая идея. Объект создаётся и больше не удаляется. В PHP это не столь важно, т.к. через доли секунды скрипт завершится и освободит все ресурсы, но стоит иметь в виду.


-----
PostgreSQL DBA
 
 Top
lodar
Отправлено: 16 Октября, 2013 - 12:53:39
Post Id


Новичок


Покинул форум
Сообщений всего: 20
Дата рег-ции: Авг. 2009  


Помог: 0 раз(а)
Мелкий пишет:
Мне кажется логичным указать это в комментарии ко всему классу. Что от использования класса можно ожидать исключение с таким-то именем.
Здесь надо решить, что важнее - документация кода или усилия, на это затрачивающиеся. Если комментарий надо исправить в двух местах - когда-нибудь это будет не сделано. А противоречивая документация хуже её отсутствия.

Подумаю. Учту.

Мелкий пишет:

Хочу заметить по непосредственно коду - у вас очень загадочным образом размазаны настройки подключения. Почему dsn - в конструкторе, а логин и пароль - в подключении?
И раз класс явно называется DBSettings, то почему getDSN не в нём?

DSN - это же уже требование PDO. Поэтому его формированием занимается класс ответственный за соединение (создание экз. PDO). DBSetting чистый контейнер для входных данных, таких как имя юзера, бд и тд. Хотя возможно вы и правы.

Мелкий пишет:

Почему сделано getProperty я понимаю, удобство обращения к массиву. Но почему бы для явным образом названного класса не повесить публичные getUsername, getPassword? Они лаконичнее, а изнутри могут проксировать к getProperty.

Создавал на скорую руку. Идея такая посещала. Переделаю.

Мелкий пишет:

Ну и кидать исключения в конструкторе - не самая лучшая идея. Объект создаётся и больше не удаляется. В PHP это не столь важно, т.к. через доли секунды скрипт завершится и освободит все ресурсы, но стоит иметь в виду.


Учту. Не подумал.

А вообще я либу пишу для себя. За основу взял из фреймворка Yii. Выложил на github. Был бы очень признателен в любой критике.

(Отредактировано автором: 16 Октября, 2013 - 13:28:54)

 
 Top
lodar
Отправлено: 17 Октября, 2013 - 03:03:43
Post Id


Новичок


Покинул форум
Сообщений всего: 20
Дата рег-ции: Авг. 2009  


Помог: 0 раз(а)
Мелкий пишет:
И раз класс явно называется DBSettings, то почему getDSN не в нём?
Почему сделано getProperty я понимаю, удобство обращения к массиву. Но почему бы для явным образом названного класса не повесить публичные getUsername, getPassword? Они лаконичнее, а изнутри могут проксировать к getProperty.

Поправил, исходя из вашего замечания. В общем жду критику, и хотелось бы побольше))). Спасибо.
 
 Top
tato
Отправлено: 17 Октября, 2013 - 04:37:08
Post Id



Посетитель


Покинул форум
Сообщений всего: 468
Дата рег-ции: Сент. 2011  
Откуда: Владивосток


Помог: 8 раз(а)
PHP:
скопировать код в буфер обмена
  1.  
  2. foreach ($parts as $i => $part)
  3.     $parts[$i] = $this->quoteSimpleTableName($part);
  4.  


PHP:
скопировать код в буфер обмена
  1.  
  2. array_walk( $parts, array( $this, 'quoteSimpleTableName' ) );
  3.  


Есдинственнное метод quoteSimpleTableName() должен принимать параметр по ссылке

PHP:
скопировать код в буфер обмена
  1.  
  2. public function quoteSimpleTableName( &$param ){ ... }
  3.  


& - амперсанд надо поставить.
(Добавление)
PHP:
скопировать код в буфер обмена
  1.  
  2. protected function getProperty($key) {
  3.     return $this->settings[$key];
  4. }
  5.  


PHP:
скопировать код в буфер обмена
  1.  
  2. protected function getProperty( $key )
  3. {
  4.     return isset( $this->settings[$key] ) ? $this->settings[$key] : 'NOT SET';
  5. }
  6.  


Могут возникнуть проблеммы если нет такого значения.
(Добавление)


Вообщем в лику напиши если интересно.


-----
просто ?: сложно
 
 Top
lodar
Отправлено: 17 Октября, 2013 - 10:18:37
Post Id


Новичок


Покинул форум
Сообщений всего: 20
Дата рег-ции: Авг. 2009  


Помог: 0 раз(а)
tato пишет:
Могут возникнуть проблеммы если нет такого значения.

Я думал над этим, но потом решил, что если такого параметра не будет, то пусть лучше. возвращаться null. B док блоке описал @return соответствующий. На него и проверять (is_null), вместо "NOT SET". Или лучше вот так явно проверку проводить?

tato пишет:
array_walk( $parts, array( $this, 'quoteSimpleTableName' ) );

Чем лучше?

(Отредактировано автором: 17 Октября, 2013 - 10:46:20)

 
 Top
Мелкий Супермодератор
Отправлено: 17 Октября, 2013 - 11:23:36
Post Id



Активный участник


Покинул форум
Сообщений всего: 11926
Дата рег-ции: Июль 2009  
Откуда: Россия, Санкт-Петербург


Помог: 618 раз(а)
lodar пишет:
Я думал над этим, но потом решил, что если такого параметра не будет, то пусть лучше. возвращаться null.

Зависит от желаемого поведения. NOT SET - точно дурацкая идея, это потом по коду будут понапиханы проверки на равенство NOT SET. А у вас не хватает проверки на существование элемента, чтобы вернуть null. Иначе помимо null будет генерироваться E_NOTICE.

Чаще встречается такой вариант:
PHP:
скопировать код в буфер обмена
  1. protected function getProperty( $key, $sDefaultValue = null )
  2. {
  3.     return isset( $this->settings[$key] ) ? $this->settings[$key] : $sDefaultValue;
  4. }

Это позволяет и легко находить не настроенные параметры и лаконично обрабатывать необязательные.
Иногда предпочтительнее выбросить исключение, если элемент жизненно важен.


-----
PostgreSQL DBA
 
 Top
lodar
Отправлено: 17 Октября, 2013 - 22:40:22
Post Id


Новичок


Покинул форум
Сообщений всего: 20
Дата рег-ции: Авг. 2009  


Помог: 0 раз(а)
Мелкий пишет:
Это позволяет и легко находить не настроенные параметры и лаконично обрабатывать необязательные.
Иногда предпочтительнее выбросить исключение, если элемент жизненно важен

Спасибо. Учел.
 
 Top
tato
Отправлено: 18 Октября, 2013 - 02:47:34
Post Id



Посетитель


Покинул форум
Сообщений всего: 468
Дата рег-ции: Сент. 2011  
Откуда: Владивосток


Помог: 8 раз(а)
Мелкий пишет:
NOT SET - точно дурацкая идея

NOT SET имел ввиду как случай, а не строкой забивать.
(Добавление)
lodar пишет:
Чем лучше?


Если есть вызов функции в foreach, то лучше использовать array_walk, т.к. он для этого и предназначен. Код меньше и понятнее. array_walk работает медленнее, но в современных реалиях, если вы не прогоняете массив в over9000 записей, то не почувствуете разницы.


-----
просто ?: сложно
 
 Top
Поиск в теме | Версия для печати
Страниц (1): [1]
Сейчас эту тему просматривают: 0 (гостей: 0, зарегистрированных: 0)
« Вопросы новичков »


Все гости форума могут просматривать этот раздел.
Только зарегистрированные пользователи могут создавать новые темы в этом разделе.
Только зарегистрированные пользователи могут отвечать на сообщения в этом разделе.
  



Powered by PHP  Powered By MySQL  Powered by Nginx  Valid CSS  RSS

 
Powered by ExBB FM 1.0 RC1. InvisionExBB