views:

48

answers:

1

As a sysadmin, I end up doing some simple ad-hoc programming every once in a while. I'm trying to learn as I go along, so in general, is there anything in the code below that jumps out at you as being bad practise or otherwise unnecessary?

Specifically, the 3 if statements at the end feels like I'm duplicating code unnecessarily. Is there any way to shorten it further without going overboard with complexity?

<?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);

?>
+3  A: 

You're not repeating unnecessarily, but I was confused because search_whois doesn't take a domain.

I'd reorganize so search_whois is self-contained

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;
}
bemace
Thank you, that seems way more elegant. Now I just need to figure out why I didn't see the solution as such in the first place. Hopefully that comes with experience. :)
Xhantar
@Xhantar - eventually you would have wanted to check several domains at once, at which point the function would have become obvious. Repeat that cycle enough times and you'll start to see it in advance.
bemace