1. PHP / Говнокод #5188

    +161

    1. 01
    2. 02
    3. 03
    4. 04
    5. 05
    6. 06
    7. 07
    8. 08
    9. 09
    10. 10
    11. 11
    12. 12
    13. 13
    14. 14
    15. 15
    16. 16
    17. 17
    18. 18
    19. 19
    20. 20
    21. 21
    22. 22
    23. 23
    24. 24
    <?php
...
	public function update() {
		if($this->data['id'])
			return false;
		
		$values = array();
		
		foreach($this->data AS $key => $value) {
			if($key == 'id')
				continue;
			
			$values[] = "`{$key}` = " . ($value ? "'{$value}'" : "NULL");
		}
		
		$result = $this->sql->query("
			UPDATE `users`
			SET (".implode(', ' $values).")
			WHERE `id` = '{$this->data['id']}'");
		
		return (bool) $this->sql->affected_rows;
	}
...
?>

    Моё

    Запостил: Devzirom, 09 Января 2011

    Комментарии (15) RSS

    • ava mindfreakthemon 09.01.2011 11:58 # +1 

      if($this->data['id'])
			return false;

      А тут ! случаем не пропущен?
      Иначе ведь запросы будут выполнятся только для id = 0. (для NULL, false будет ошибка)
      Ответить

      • ava Devzirom 09.01.2011 12:31 # 0

        Действительно ошибка!

        if(!$this->data['id'])
        return false;
        Ответить

    • ava KirAmp 09.01.2011 12:10 # −1

      Devzirom,
      Вы слишком самокритичны, беглый осмотр говна не выявил (разве только что это php) =)
      Ответить

      • ava Devzirom 09.01.2011 12:37 # 0

        Я думаю, что существует более корректный способ вставить ассоциированный массив в запрос
        Ответить

    • ava Lure Of Chaos 09.01.2011 12:40 # −2

      а зачем из ассоциативного лепить обычный? что бы его имплодить?
      думаю, стоило бы сразу лепить в строку в цикле
      Ответить

      • ava Devzirom 09.01.2011 12:45 # 0

        Всё дело в последней запятой. Пришлось бы проверять является ли элемент массива последним. Вот тогда бы и появились явные признаки говнокода
        Ответить

        • ava Lure Of Chaos 09.01.2011 13:14 # 0 

          $values = '';
foreach($this->data as $key => $value) {
if($key == 'id') continue;
$values. =($values?',':'').'`'.$key.'` = ' . ($value ? '\''.$value.'\'' : 'NULL');
}

          и без имплода
          Ответить

        • ava striker 09.01.2011 17:13 # 0 

          foreach (...) {
    $values .= $key . ' = ' . $value . ', ';
}
$values = substr($values, 0, -2);

          и, кстати, не забудьте про квотинг переменных в sql запросах
          Ответить

          • ava Devzirom 09.01.2011 18:34 # 0

            Он вроде как бы есть
            Ответить

          • ava eth0 09.01.2011 19:19 # −1

            Вот интересно. Обрезание последней запятой - грязный хак или нет?
            Ответить

            • ava Lure Of Chaos 09.01.2011 19:35 # −1

              в принципе так быстрее работать должно, операция вынесена за пределы цикла
              Ответить

            • ava xXx_totalwar 09.01.2011 19:43 # +1

              лол) по сравнению с самим подходом (велик вместо полноценного орм) - это капля в море
              Ответить

    Добавить комментарий