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

    +150

    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
    <?php
	require_once("inc/mysql.php");
	sleep(3); //Для ajax запроса, потом удалить

	// Проверяем, что к нам идёт Ajax запрос
	if (isset($_SERVER['HTTP_X_REQUESTED_WITH']) && !empty($_SERVER['HTTP_X_REQUESTED_WITH']) && strtolower($_SERVER['HTTP_X_REQUESTED_WITH']) == 'xmlhttprequest') {
		$city1 = $_POST['city1'];
	}else{
		exit(); // Заканчиваем работу скрипта, если это не ajax запрос
	}

	if (isset($city1)) {
		$query = "SELECT id_city, id_region, id_country FROM cities WHERE city_name_ru LIKE '$city1'";
		$result = mysqli_query($link, $query);// or trigger_error(mysql_error($link)." ".$query);
		$row = mysqli_fetch_array($result);
		$country = $row['id_country'];
		//echo $country;
		//echo '<br>';
		$region = $row['id_region'];
		//echo $region;
		//echo '<br>';
		$query1 = "SELECT country_name_ru FROM countries WHERE id_country = '$country'";
		$result1 = mysqli_query($link, $query1);// or trigger_error(mysql_error($link)." ".$query);
		$row1 = mysqli_fetch_array($result1);
		echo '<p class=\'country\'>Страна '.$row1['0'].'</p>';
		//echo '<br>';
		$query2 = "SELECT region_name_ru FROM regions WHERE id_region = '$region'";
		$result2 = mysqli_query($link, $query2);// or trigger_error(mysql_error($link)." ".$query);
		$row2 = mysqli_fetch_array($result2);
		echo '<p class=\'reqion\'>Область '.$row2['0'].'</p>';
	}
?>

    Прокомментируйте, пожалуйста, как улучшить этот говнокод.

    Запостил: eprivalov1, 25 Апреля 2014

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

    • ava Dart_Sergius 25.04.2014 12:07 # 0

      ИМХО :
      - вынести работу с mysql в класс или функции
      -убрать sleep, ибо он нафиг не нужен.
      -и в AJAX возвращать чистый html код - это потенциальная брешь в безопасности.
      Ответить

      • ava bormand 25.04.2014 12:10 # +3

        > убрать sleep, ибо он нафиг не нужен
        У меня в бекендовом демоне до сих пор стоят sleep(5), ибо без них все слишком быстро работает, и даже как-то некрасиво - не успеваешь разглядеть какие этапы проходят, и как вертится индикатор около текущего пункта ;(
        Ответить

        • ava bormand 25.04.2014 12:59 # 0

          P.S. Ну правда там sleep() в воркере, а не во время обработки ajax-запроса :) Так что код ОП'а поговнистей будет.
          Ответить

          • ava Dart_Sergius 25.04.2014 13:10 # 0

            ИМХО - задержка, дело не серверной части, это уже клиентская часть, для нашего удобства. А обработка на сервере должна проходить максимально быстро, и лишний раз не занимать процессорное время.
            Ну это как я вижу этот мир.
            Ответить

            • ava bormand 25.04.2014 13:23 # +1

              > занимать процессорное время
              Ты так говоришь, как-будто sleep() его занимает ;)

              Но да, эти sleep'ы стоит выпилить, чтобы юзеру меньше ждать приходилось.
              Ответить

              • ava WGH 25.04.2014 14:06 # +1

                > Ты так говоришь, как-будто sleep() его занимает ;)
                Но зато рабочий процесс/тред апача/php-fpm/etc. простаивает без дела, в то время как другие клиенты, возможно, ждут освобождения этого процесса.
                Ответить

                • ava bormand 25.04.2014 15:08 # 0

                  Ну если доебаться к словам - это уже не процессорное время, а wall clock (как это красиво сказать по-русски?).
                  Ответить

                  • ava WGH 25.04.2014 15:26 # 0

                    Я и не говорю, что это процессорное время. Я лишь хочу сказать, что висящий на sleep процесс PHP - это отнюдь не бесплатно.
                    Ответить

                    • ava bormand 25.04.2014 15:37 # 0

                      > висящий на sleep процесс PHP
                      Если быть точным, то там такая схема:
                      1) Сам демон написан на змее, пыхоморда общается с ним через json, а аяксовые приблуды для индикации статуса - через пыхоморду
                      2) Воркер - внешний процесс, запущенный демоном, который по ходу работы передает ему список этапов с флажками done/active/pending и список ошибок/предупреждений. В него и воткнуты sleep'ы...

                      Оверинжиниринг детектед? :)

                      Небесплатность sleep'а признаю, и втыкание sleep'а "для красоты" всячески порицаю ;)
                      Ответить

        • ava bormand 25.04.2014 13:01 # +8

          А потом можно будет вырезать эти sleep'ы и писать в чейнжлоге - существенно ускорена работа таких-то операций.
          Ответить

      • ava eth0 25.04.2014 18:40 # +2

        Это всё равно что посоветовать экономить на спичках, хотя тут вовсю инъекция хлещет, три запроса вместо джойнов и стиль вмазанного овоща.
        Больше прохладных советов, бро!
        Ответить

    • ava eth0 25.04.2014 18:36 # 0

      Переписать на руби!
      Ответить

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