Is this code tidy and neat enougth?

PHP programming forum. Ask questions or help people concerning PHP code. Don't understand a function? Need help implementing a class? Don't understand a class? Here is where to ask. Remember to do your homework!

Moderator: General Moderators

Post Reply
127.0.0.1
Forum Newbie
Posts: 22
Joined: Mon Sep 12, 2005 8:59 pm

Is this code tidy and neat enougth?

Post 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 :)
User avatar
John Cartwright
Site Admin
Posts: 11470
Joined: Tue Dec 23, 2003 2:10 am
Location: Toronto
Contact:

Post 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..
User avatar
pickle
Briney Mod
Posts: 6445
Joined: Mon Jan 19, 2004 6:11 pm
Location: 53.01N x 112.48W
Contact:

Post 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.
Real programmers don't comment their code. If it was hard to write, it should be hard to understand.
127.0.0.1
Forum Newbie
Posts: 22
Joined: Mon Sep 12, 2005 8:59 pm

Post 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
pilau
Forum Regular
Posts: 594
Joined: Sat Jul 09, 2005 10:22 am
Location: Israel

Post by pilau »

Jcart, will that syntax work in PHP4 too?
127.0.0.1
Forum Newbie
Posts: 22
Joined: Mon Sep 12, 2005 8:59 pm

Post 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.
pilau
Forum Regular
Posts: 594
Joined: Sat Jul 09, 2005 10:22 am
Location: Israel

Post 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
127.0.0.1
Forum Newbie
Posts: 22
Joined: Mon Sep 12, 2005 8:59 pm

Post 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.
127.0.0.1
Forum Newbie
Posts: 22
Joined: Mon Sep 12, 2005 8:59 pm

Post 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);
pilau
Forum Regular
Posts: 594
Joined: Sat Jul 09, 2005 10:22 am
Location: Israel

Post by pilau »

Looks good to me, well done.
timvw
DevNet Master
Posts: 4897
Joined: Mon Jan 19, 2004 11:11 pm
Location: Leuven, Belgium

Post 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...
127.0.0.1
Forum Newbie
Posts: 22
Joined: Mon Sep 12, 2005 8:59 pm

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