Избегайте дублирования кода (PHP)

Как системный администратор, я время от времени выполняю простое специальное программирование. Я пытаюсь учиться по мере продвижения, так что в целом, есть ли в приведенном ниже коде что-то, что бросается вам в глаза как плохая практика или иным образом ненужное?

В частности, 3 оператора if в конце выглядят так, будто я излишне дублирую код. Есть ли способ сократить его еще больше, не переборщив со сложностью?

<?php

define('TAKEN', 'Match: One');
define('AVAIL', 'Match: No Matches');
define('DATAMINE', 'Data mining count exceeded');

$ch = curl_init("http://co.za/cgi-bin/whois.sh?Domain=example");

curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
curl_setopt($ch, CURLOPT_HEADER, 0);

$output = curl_exec($ch);

function search_whois($findit) {
        global $output;
        if (strpos($output, $findit) === false)
                    return false;
        if (is_int(strpos($output, $findit)))
                return true;
}

if (search_whois(TAKEN))
        echo "Domain is taken.\n";

if (search_whois(AVAIL))
        echo "Domain is available.\n";

if (search_whois(DATAMINE))
        echo "Blocked for datamining, try again later.\n";

// var_dump($output);

?>

php code-duplication
person Xhantar    schedule 15.10.2010    source источник
comment
Мне нравится видеть, как разработчики действительно хотят улучшить свой собственный код и хотят делать все правильно. Я дам вам +1 за это! Поскольку сейчас поздний вечер пятницы, я, вероятно, не тот человек, чтобы проверять чужой код прямо сейчас. Тем не менее, я не вижу ничего явно неправильного. Если последние три if являются взаимоисключающими, вместо этого вы можете использовать if-else if, но это придирки.   -  person kskjon    schedule 15.10.2010
comment
Как упоминалось в комментарии выше, все 3 IF будут работать, если это необходимо, то с вашим кодом все в порядке, если вместо этого вы хотите завершить/выйти после каждого if или разрешить запуск только 1 из них, затем используйте if/elseif/else так далее...   -  person Viper_Sb    schedule 15.10.2010
comment
Единственное, что я мог бы сделать по-другому, это создать класс вместо простого использования функции, поэтому вам не нужно использовать global $output. В остальном, я думаю, ты довольно солидный.   -  person tplaner    schedule 15.10.2010
comment
Да, они взаимоисключающие. Домен либо занят, либо доступен, либо поиск был заблокирован, поэтому вместо этого имеет смысл использовать if-elseif-else. Спасибо за ваши комментарии!   -  person Xhantar    schedule 15.10.2010
comment
@evolve: да, мне было интересно, не будет ли лучше просто передать $output функции в качестве аргумента, вместо того, чтобы делать ее глобальной (кажется, я где-то читал, что следует избегать использования глобальных переменных, если это возможно, хотя я не знаю почему - нужно еще немного почитать.) В любом случае, передача всей строки $output в функцию казалась... снова расточительной (она уже там, не так ли?), поэтому я решил пойти с глобальным идея.   -  person Xhantar    schedule 15.10.2010

Ответы (1)


arrow_upward
3
arrow_downward

Вы зря не повторяетесь, но я был сбит с толку, потому что search_whois не занимает домен.

Я бы реорганизовал так, чтобы search_whois был автономным

function search_whois($domain) {
    $ch = curl_init("http://co.za/cgi-bin/whois.sh?Domain=$domain");

    curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
    curl_setopt($ch, CURLOPT_HEADER, 0);

    $output = curl_exec($ch);

    if (strpos($output, AVAIL) >= 0) {
        echo "Domain is available.\n"
        return true;
    }

    if (strpos($output, TAKEN) >= 0)
        echo "Domain is taken.\n";
    else if (strpos($output, DATAMINE) >= 0)
        echo "Blocked for datamining, try again later.\n"

    return false;
}
person Brad Mace    schedule 15.10.2010
comment
Спасибо, это кажется более элегантным. Теперь мне просто нужно выяснить, почему я не видел решения как такового в первую очередь. Надеюсь, это приходит с опытом. :) - person Xhantar; 15.10.2010
comment
@Xhantar - в конце концов вы захотели бы проверить несколько доменов одновременно, и в этот момент функция стала бы очевидной. Повторите этот цикл достаточное количество раз, и вы начнете видеть его заранее. - person Brad Mace; 15.10.2010