Page 1 of 1

Is this code tidy and neat enougth?

Posted: Sat Oct 01, 2005 11:43 am
by 127.0.0.1

Code: Select all

$URL = explode('/',$_POST['url']); // This just strips i.e http://www.site.com/yes.php to http://www.site.com
	
$UrlCheck = gethostbyname($URL[0]);
	
if($UrlCheck == $_POST['url'])
	{
	$UrlCheckMessage = "FAIL --- Unable to resolve IP";
	}
	
else	{
	$UrlCheckMessage = "PASS --- IP = $UrlCheck";
	}
the idea of the script is that if gethostbyname returns an ip then its a valid url else its not valid.

Is this an acceptable way to write this?

I am looking for pointers to improve my code :)

Posted: Sat Oct 01, 2005 11:47 am
by John Cartwright

Code: Select all

$URL = explode('/',$_POST['url']);
$UrlCheck = gethostbyname($URL[0]);    
$UrlCheckMessage = ($UrlCheck == $_POST['url'] ? 'Fail -- Unable to resolve IP' : 'Pass -- IP is '.$UrlCheck);
Probably would want to validate $_POST['url'] to make sure it is in the proper format though..

Posted: Sat Oct 01, 2005 11:55 am
by pickle
Your code could be a bit neater and consistent (I wouldn't even mention it though if you hadn't asked). The way you've laid out your if/else statement is a) slightly inconsistent and b) not 100% conforming to pre-existing practices.

One practice is almost exactly like you've got the else clause set up - with the opening bracket on the same line as the condition:

Code: Select all

if($UrlCheck == $_POST['url']){
    $UrlCheckMessage = "FAIL --- Unable to resolve IP";
}
    
else{
    $UrlCheckMessage = "PASS --- IP = $UrlCheck";
}
The method I personally prefer is to put both the opening and closing brackets on their own line:

Code: Select all

if($UrlCheck == $_POST['url'])
{
    $UrlCheckMessage = "FAIL --- Unable to resolve IP";
}
else
{
    $UrlCheckMessage = "PASS --- IP = $UrlCheck";
}
Most important is consistency. It's very difficult to read large blocks of code with no consistent style. As long as code has a consistent style, it's OK to read.

Posted: Sat Oct 01, 2005 11:57 am
by 127.0.0.1
Yeah the way i see is that if they input the url incorrectly then its going to produce an error. Asin validate the url i think you mean check that theres a www and a . etc things like that?

I have already set the value of the input box to the right format(value="www.").

I like the way you wrote it :) very simple but i think sometimes its best to ask people who have been coding a while how they would have done it and then compare.

Thanks

Posted: Sat Oct 01, 2005 11:59 am
by pilau
Jcart, will that syntax work in PHP4 too?

Posted: Sat Oct 01, 2005 12:02 pm
by 127.0.0.1
Yeah i think it broke the formatting when i copied paste... in an if statement or loop... i always put a { and } on a new line with one tab from the previous {

for instance

Code: Select all

if($day == "MONDAY")
      {
      if($time == 4)
              {
              echo "wake up";
              }
      }
I know above code could be written in one if statement or even another way... just to show you where i place my brackets. Is this bad practice?. I find it easier to read this way with each block in more of a section.

Posted: Sat Oct 01, 2005 12:14 pm
by pilau
127.0.0.1 wrote:I have already set the value of the input box to the right format(value="www.").
Not enough.
Consider that there are also sub domains which don't start with a "www".

Besides, you should check for string.string.extension

Posted: Sat Oct 01, 2005 12:24 pm
by 127.0.0.1
mabye i should use parse_url ?

I will work on something then post it and get your views on it.

Posted: Sat Oct 01, 2005 12:44 pm
by 127.0.0.1
Ok, This code seems to work fine with subdomains and you can enter the url in http://yoursite.com and http://www.yoursite.com.

gethostbyname(); will only work with the url in the format of http://www.yourdomain.com or cp.domain.com

Code: Select all

$URL = explode('/',ereg_replace("http://","",strtolower($_POST['url']))); 
$UrlCheck = gethostbyname($URL[0]);     
$UrlCheckMessage = ($UrlCheck == $_POST['url'] ? 'Fail -- Unable to resolve IP' : 'Pass -- IP is '.$UrlCheck);

Posted: Sat Oct 01, 2005 1:12 pm
by pilau
Looks good to me, well done.

Posted: Sat Oct 01, 2005 1:20 pm
by timvw
it appears you are trying to test if the url is valid/working.. meaby a http HEAD request might be more appropriate..
I believe there is a pear package, HTTP_* package that can do this for you...

Posted: Sat Oct 01, 2005 4:19 pm
by 127.0.0.1
I dont feel I would learn from using pear pre made scripts.

I feel that to be copy and paste, thats ok if you have deadlines to meet I think but I am just learning to code. All i would need to do to return the headers is use curl.

I might code a little script that returns the header for a domain and get your views on it.

I hope you guys dont mind me asking is this code is good or crap. :)