HardCode
115 повідомлень
#15 років тому
Это можно назвать трудночитаемым кодом? 
if(sizeof($quickMethods = $component->quickMethods()) > 0)
$this->registerQuickMethods($class, $quickMethods);
или такой
public function __get($name)
{
if(isset($this->_components))
return $this->_components;
else if(isset($this->components))
return $this->_components = $this->registerComponent($name, $this->components);
parent::__get($name);
}
Я так часто пишу, просто не нравится делать две строчки если для меня и в одну нормально смотрится...Но все же как это..плохо?
115 повідомлень
#15 років тому
И так 
static public function t($name, $key)
{
if(isset(self::$_messages))
$message = self::$_messages;
else if(is_file($messageFile = self::getPath('messages.' . $name, 'php')))
$message = self::$_messages = include($messageFile);
else
throw new DlException('Unable to load message file "{name}".', array('{name}' => $messageFile));
if(isset($message))
return $message;
return $key;
}
$expire = time() + ($expire === 0 ? 31536000 : $expire);
3240 повідомлень
#15 років тому
Цитата ("trueW3C"):Это можно назвать трудночитаемым кодом?
Да, это можно назвать трудночитаемым кодом, но смотря как оценивать-то.
Такой вопрос вообще настолько спорный, что однозначно что-то нельзя утверждать. У всех же свои критерии, некоторые например считают, что чем меньше строк кода используется для решения задачи, и чем больше их помещается на экране одновременно, тем лучше.
Я к таким людям не отношусь, хотя и понимаю их позицию, и не осуждаю, но этот код переписал бы, если бы хотел добиться читаемости (примечание: если бы заботился о производительности, вообще бы не использовал бы __get):
$quickMethods = $component->quickMethods();
if (sizeof($quickMethods) > 0) {
$this->registerQuickMethods($class, $quickMethods);
}
Это и немного проще (нагляднее), и, что важно, в процессе отладки удобно вставлять вызов логгера после присваивания того же $quickMethods.
public function __get($name)
{
if (isset($this->_components)) {
return $this->_components;
}
if (isset($this->components)) {
$component = $this->registerComponent($name, $this->components);
$this->_components = $component;
return $component;
}
parent::__get($name);
}
Обратите внимание, в Вашем решении Вы используете if (...) return ... else if (...), в этом случае else избыточно, то есть просто не нужно даже по логике.
Далее, конструкция return a = b весьма хороша и компактна, но я все же разбил ее на составные части, так как тоже проще осмыслить что там происходит, не будет путаницы с = и ==, и в случае отладки проще туда вставлять вызовы логгера (а вот с точки зрения производительности, а не читаемости кода и удобства для программиста, конечно, лучше было бы return a = b, как Вы и сделали, но вряд ли об производительности идет речь с таким подходом с __get и кучей обращений к хешам).
И еще, я бы сильно бы задумался над почти совпадающими наименованиями _components и components...
115 повідомлень
#15 років тому
Цитата ("tvv"):в этом случае else избыточно, то есть просто не нужно даже по логике.
Вы не правы. В вашем варианте интерпретатор всегда два раза будет проверять условия, а в моем варианте...второе условие будет проверяться только если не выполнится первое.
Цитата ("tvv"):
но вряд ли об производительности идет речь с таким подходом с __get
Речь об удобстве...заранее не известно, что грузится и сразу всё грузить нет смысла. Тут один хеш... $this->_components, а $this->components это конфиг который меняется уже как надо...
'components' => array(
'router' => array(
'class' => 'DlRouter',
'routes' => require(Dl::getPath('core.config.routes', 'php')),
),
'plugins' => array(
'class' => 'DlPlugins',
'autoload' => true,
),
'view' => array(
'class' => 'DlView',
),
'user' => array(
'class' => 'DlUser',
),
'db' => array(
'class' => 'DlDb',
'connectionString' => 'mysql:host=localhost;dbname=dlife',
'username' => 'root',
'password' => '',
),
),
3240 повідомлень
#15 років тому
Цитата ("trueW3C"):$expire = time() + ($expire === 0 ? 31536000 : $expire);
Такая конструкция имеет право на жизнь, в том числе и с точки зрения читаемости. Хотя я бы так не делал бы, но не имею ничего против.
Хотя значение 31536000 лучше бы вынести в константы (если, конечно, не стоит задача считать микросекунды, а цель добиться читаемости кода).
И хорошо бы подумать над именами идентификаторов, так как $expire в левой части этого выражения это по смыслу совсем другое, чем $expire в правой части. То есть слева это время от текущего, а справа, по логике — интервал в секундах.
Наверное лучше было бы как-то так:
$expireTime = time() + ($expireInterval === 0 ? MAX_EXPIRATION_INTERVAL : $expireInterval);
3240 повідомлень
#15 років тому
Цитата ("trueW3C"):Вы не правы. В вашем варианте интерпретатор всегда два раза будет проверять условия, а в моем варианте...второе условие будет проверяться только если не выполнится первое.
Вы уверены? Мой вариант полностью аналогичен Вашему, но только он без избыточного else. Там же return вызывается.
Кстати, это избавление от else, в случае с return — это один из стандартных методов рефакторинга для таких случаев, описанный в литературе по рефакторингу.
115 повідомлень
#15 років тому
Цитата ("tvv"):Там же return вызывается.
Ой. Извините, уже сплю
Поправил у себя в кодеЕщё кусок тяжелого кода
Вот некрасиво смотрится если unset на новой строчке$class = $params; unset($params);
$component = new $class($params);
3240 повідомлень
#15 років тому
Цитата ("trueW3C"):static public function t($name, $key)
Так функции лучше не называть. Желательно давать осмысленные названия. Так как нелегко читать код, где куча вызовов функций a(), t(), z() и т.д.
115 повідомлень
3240 повідомлень
#15 років тому
Цитата ("trueW3C"):Ещё кусок тяжелого кода$class = $params; unset($params);
$component = new $class($params);
Да, тут все плохо. Сходу разобраться в этом коде человеку со стороны будет сложно. Логика непонятная.
Скорее всего тут проблема в дизайне используемых классов, я не вижу всего кода, но не уверен, что тут нет явного нарушения LoD.
И таки да, желательно каждую операцию писать в отдельной строке.