Avoiding duplicate code (PHP)

Question

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

?>

I love seeing developers actually wanting to improve their own code and wanting to do things properly. I'll give you a +1 for that! Since it's late Friday afternoon, I'm probably not the right person to validate someone elses code right now. However, I can't see anything obviously wrong. If the last three if's are mutually exclusive, you could use if-else if instead, but that is nitpicking. — user466784
– user466784, Commented Oct 15, 2010 at 13:05
As mentioned in the comment above all 3 IFs will run, if this is wanted then ya your code is fine, if instead you want to end/exit after each if, or only allow 1 of them to run then use if/elseif/else etc... — Viper_Sb
– Viper_Sb, Commented Oct 15, 2010 at 13:07
The only thing I might do differently is make a class instead of simply just using a function so you don't have to use global $output. Other than that, I think you're pretty solid. — tplaner
– tplaner, Commented Oct 15, 2010 at 13:15
Yes, they're mutually exclusive. The domain is either taken, or available, or the lookup was blocked, so using if-elseif-else instead makes sense. Thanks for your comments! — Xhantar
– Xhantar, Commented Oct 15, 2010 at 13:18
@evolve: yeah, I was wondering if it wouldn't be better to just pass $output to the function as an argument instead of making it global (I think I read somewhere that using global vars should be avoided, if possible, although I'm not sure why - need to do some more reading.) Anyway, passing the whole $output string to the function seemed.. wasteful again (it's already there, isn't it?), so I decided to go with the global idea. — Xhantar
– Xhantar, Commented Oct 15, 2010 at 13:27

Brad Mace · Accepted Answer · 2010-10-15 13:13:20Z

2

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;
}

answered Oct 15, 2010 at 13:13

Brad Mace

28k19 gold badges110 silver badges152 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Xhantar Over a year ago

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. :)

Brad Mace Over a year ago

@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.

Collectives™ on Stack Overflow

Avoiding duplicate code (PHP)

1 Answer 1

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

2 Comments

Your Answer

Sign up or log in

Post as a guest