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

    +152

    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
    25. 25
    26. 26
    27. 27
    28. 28
    29. 29
    30. 30
    31. 31
    32. 32
    33. 33
    34. 34
    35. 35
    36. 36
    37. 37
    38. 38
    39. 39
    40. 40
    41. 41
    42. 42
    43. 43
    44. 44
    45. 45
    46. 46
    47. 47
    48. 48
    49. 49
    50. 50
    51. 51
    52. 52
    53. 53
    54. 54
    55. 55
    56. 56
    57. 57
    58. 58
    59. 59
    60. 60
    function getTestDataById ($testid) {
    
    	$query="select * from mdl_test where id=".$testid."";
    	$result = mysql_query($query) or die('getTestDataById query failed: ' . mysql_error());
    
    	while ($row = mysql_fetch_array($result, MYSQL_ASSOC)) {
    
    	$courseid=$row['courseid'] ;
    	$moduleid=$row['moduleid'];
    	$name=$row['name'];
    	$maxscore=$row['maxscore'];
    	$successscore=$row['successscore'];
    	$attempts=$row['attempts'];
    	$dur=$row['dur'];
    	$showsuccessmessage=$row['showsuccessmessage'];
    	$successmessage=$row['successmessage'];
    	$showfailedmessage=$row['showfailedmessage'];
    	$failedmessage=$row['failedmessage'];
    	$showtestsuccessmessage=$row['showtestsuccessmessage'];
    	$testsuccessmessage=$row['testsuccessmessage'];
    	$showtestfailedmessage=$row['showtestfailedmessage'];
    	$testfailedmessage=$row['testfailedmessage'];
    	$freequestions=$row['freequestions'];
    	$questionsorder=$row['questionsorder'];
    	$defertest=$row['defertest'];
    	$totalperpage=$row['totalperpage'];
    	$showcorrectreply=$row['showcorrectreply'];
    	$showscore=$row['showscore'];
    	$limittype = $row['limittype'];
    	$annotation = $row['annotation'];
    
    	} // end while
    
    	$res=array('courseid'=>$courseid,
    			   'moduleid'=>$moduleid,
    			   'name'=>stripslashes($name),
    			   'maxscore'=>$maxscore,
    			   'successscore'=>$successscore,
    			   'attempts'=>$attempts,
    			   'dur'=>$dur,
    			   'showsuccessmessage'=>$showsuccessmessage,
    			   'successmessage'=>stripslashes($successmessage),
    			   'showfailedmessage'=>$showfailedmessage,
    			   'failedmessage'=>stripslashes($failedmessage),
    			   'showtestsuccessmessage'=>stripslashes($showtestsuccessmessage),
    			   'testsuccessmessage'=>stripslashes($testsuccessmessage),
    			   'showtestfailedmessage'=>stripslashes($showtestfailedmessage),
    			   'testfailedmessage'=>stripslashes($testfailedmessage),
    			   'freequestions'=>stripslashes($freequestions),
    			   'questionsorder'=>stripslashes($questionsorder),
    			   'defertest'=>stripslashes($defertest),
    			   'totalperpage'=>stripslashes($totalperpage),
    			   'showcorrectreply'=>stripslashes($showcorrectreply),
    			   'showscore'=>stripslashes($showscore),
    				'limittype' => stripslashes($limittype),  // Total 21 items
    				'annotation' => stripslashes($annotation));
    
    	return $res;
    
    	} // end of function

    От начальника отдела))) Как вам?))

    Запостил: Krugly, 09 Марта 2013

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

    • Возвращает последнюю случайную (т.к. нет order by) запись из тех, у которых id = $testid?
      Ответить
      • Обычно id -> обзывают автоинкрементное поле в табличках. То есть, полагаю, что запрос возвратит одну запись. Это раз. Во-вторых, наврядли без ORDER BY RAND() результат будет случайным. Скорее вернутся строки отсортированные по внутреннему порядку в таблице.

        А вообще, начальник на то и начальник, чтобы в кодах оставался на уровне времени скачка по карьерной лестнице. У него сейчас другие орг. проблемы.

        Учитывая первое предположение, можно было бы остановится на 6-й строчке: return mysql_fetch_assoc($result); // это если учесть валидность данных хранящихся в БД.
        Ответить
        • > наврядли без ORDER BY RAND() результат будет случайным
          Ну под случайной имелось в виду во-первых implementation defined и во-вторых зависящее от кучи факторов. Наверное правильнее было бы сказать неопределенную запись из тех, у которых id = $testid.

          > Обычно id обзывают автоинкрементное поле в табличках
          Обычно одну запись циклом не читают... Хотя кто знает, не видя структуры базы можно спорить бесконечно ;)

          > Учитывая первое предположение, можно было бы остановится на 6-й строчке
          Да, именно так. Ну будет единственное различие, что если добавятся новые поля, то и они вернутся. Но если это существенно - проще перечислить эти поля вместо * в селекте.
          Ответить
          • Это тот начальник, который называет себя senior php-developer
            Ответить
    • харосый насяльника, бляяаать!
      Ответить
    • Очевидно - херота.
      Ответить
    • Таблица прикольно называется. :) Остальное читать даже не хочу.
      Ответить
    • Циклы не до конца освоил, похоже.
      Ответить
      • Ну почему же? Он даже поюзал цикл там, где он нафиг не нужен...
        Ответить
    • А вы не подумали, что если id -> НЕ автоинкрементное поле в таблице?
      У меня обычно это поля называлось бы так id_mdl_test
      Так что не факт что здесь есть ошибка.
      Ответить
      • Если там действительно возможно несколько записей с таким id, то какую же из них вернет этот код? Просвятите нас, о мудрейший ;)
        Правильный ответ: а хуй бы ее знал, порядок без order by зависит от множества факторов.

        > Так что не факт что здесь есть ошибка.
        Факт, факт ;) В особенности если поле не автоинкрементное.

        А вся копипаста в 8-30 и 34-56 отлично заменяется на манипуляции над row в целом. Я уж промолчу про SQL иньекцию, это было бы слишком банально.
        Ответить
      • P.S. Вы можете сказать, что код автора корректнее обрабатывает ситуацию когда строк нет, т.к. он возвращает массив с пустыми полями а не просто false. Но, имхо, в 99.99% случаев эти пустые поля нахер никому не сдались и только усложнят проверку на то, была найдена ли запись (есть вероятность, что после вызова этой функции стоит говнокод проверяющий все 23 айтема на пустоту). Ах да, если вдруг не найдется теста с заданынм id, в лог будут высраны 23 замечательных нотиса.

        Достаточно вернуть false (что автоматически произойдет если работать с $row а не с его содержимым).

        P.P.S. Если вы скажете, что автор намеренно ограничил список полей, чтобы при модификации не вернулись лишние, то я отвечу вам: "а какого ж хуя он пишет select * ?!".

        P.P.P.S. Все, кажется перечислил все возможные ответы оппонентов, и ответил на них. Можно идти спать.
        Ответить
      • P.P.P.P.S. А нет, забыл аргумент про "но код автора же прогоняет stripslashes на некоторых, но не на всех аргументах, поэтому и убрать его нельзя, и на цикл заменить тоже". Отвечу вот так: stripslashes не нужен, как впрочем и magic quotes.
        Ответить
        • Ну конечно про какие иньекции вы говорите и stripslashes. Сразу ясно что этот код сырой и написан для быстрого временного решения задачи. Я вообще не пишу на чистом ПХП, а использую фреймворк. Тогда возникает меньше таких навязчивых идей безопасности и взлома. Безопасность не нужна, говно никто не ломает. Это же вам не вконтакте.ру, который каждый школьник и школьница мечтает сломать.
          Ответить
          • Ну иногда и говно ломают, что, впрочем, ему не особо вредит ;)

            А вообще все эти мэджик квоты да аддслеши это не безопасность, это ее иллюзия и зря потраченное время. Да и применяют их обычно через жопу и не к месту...

            А фреймворки почему-то у пыхоновичков не в почете, видимо думают, что изучать фреймворк сложнее чем извергать говно. А зря.
            Ответить
            • Чужие фреймворки изучать - фи! Правильные похабешники пишут свой собственный. Пусть его труд другие изучают!
              Ответить
          • > написан для быстрого временного решения
            Да, уж 54 строки пишутся очень быстро. Быстрее чем одна. Вы какой-то бред несёте, уважаемый. Сразу видно человека обмазанного фрейвокрами с головы до ног, и совершенно не представляеющего принципов внутренней работы этих фейворков.
            Предвосхищу ваши замечания: Нет, я уважаю фареймворки, и особенно их разработчиков. Но не боготворю их, а в некоторых случаях, целесообразнее вообще не использовать фреймворки.
            Ответить
    • А почему народ минусует пост? Если это не говно, то что это ?
      Ответить
      • > Если это не говно
        Ну почему же. Оно тут в каждой строчке кроме 1,2,5,7,31,33,57-59. Вот только оно всё какое-то унылое. Порадовала только строчка 6.

        > почему народ минусует
        Ну я, например, минусую не только не говно, но и портянки, в которых концентрация интересных вещей мала.
        Ответить
      • вам интересно было бы смотреть видео, скажем, первых шагов чьего-то младенца?
        Ответить
        • Ну тут-то далеко не младенец, а начальник отдела - уважаемый человек, а вы его код даже за говно не считаете ))

          А так да согласен, в общем и целом охота более качественных говнокодов
          Ответить
          • Насчёт начальника - это со слов запостившего. Даже если он не семён, то смотреть унылоту неинтересно ни от владельца илитной веб-студии, ни от студенческой лабораторной.
            P.S. Тут уже давным-давно ведутся творческие дискуссии на тему "что такое плюс". Пока что коллективным сознательным удалось выяснить, что это не показатель говнокодовости, но какой-то who-it-are-o-meter.
            Ответить

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