Solved

make code shorter

Posted on 2011-09-07
5
293 Views
Last Modified: 2012-05-12
hello there,
I have a little script on my website so when ppl submit new wallpapers it would scan the link to see if its online or not..
I am trying to make this code shorter and more professional, when you run the script it works fine but it needs a bit work..
what would you do?


<?


function curl($linkA, $postfeilds = '') {
	global $pagel;

	$ch = curl_init($linkA);
	curl_setopt($ch, CURLOPT_URL, $linkA);
	curl_setopt($ch, CURLOPT_HEADER, 0);
	curl_setopt($ch, CURLOPT_USERAGENT, "Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.6) Gecko/2009011913 Firefox/3.0.6 (.NET CLR 3.5.30729)");
	curl_setopt($ch, CURLOPT_CONNECTTIMEOUT, 20);
	curl_setopt($ch, CURLOPT_TIMEOUT, 20);
	curl_setopt($ch, CURLOPT_FAILONERROR, 1);
	curl_setopt($ch, CURLOPT_FOLLOWLOCATION, 1);
	curl_setopt($ch, CURLOPT_RETURNTRANSFER, 1);
	if($postfeilds) {
  	curl_setopt($ch, CURLOPT_POST, 1);
  	curl_setopt($ch, CURLOPT_POSTFIELDS, $postfeilds);
	}

	$pagel=curl_exec($ch);
	curl_close($ch);
	unset($ch);
	return($pagel);
}

function check($linkB, $regex, $pattern='', $replace='') {
    if(!empty($pattern)) {
        $linkB = preg_replace($pattern, $replace, $linkB);
    }
		$pagek = curl($linkB);
    if(eregi($regex , $pagek)) {
        echo 'alive: '.$linkB."<br>\n"; flush();
    }
    elseif(!eregi($regex , $pagek)) {
         echo 'down: '.$linkB."<br>\n"; flush();
    }
}

$sites = array(
			array("megaupload\.com\/?", "(Filename:|File description:)","MU")
);

$alllinks = '
http://www.megaupload.com/?d=UM3GFUZU
http://www.megaupload.com/?d=UM3GFUZZ
http://hotfile.com/dl/129240829/ad5cc51/Windows_7_ultimate_collection_of_wallpapers.74.jpg.html
http://hotfile.com/dl/129240829/ad5cc52/Windows_7_ultimate_collection_of_wallpapers.74.jpg.html
';

$alllinks = explode(" ", $alllinks);
$alllinks = implode("\n", $alllinks);
$alllinks = explode("\n", $alllinks);

foreach($alllinks as $linkC) {

  if(eregi("(http|www)" , $linkC)) {
  	
		foreach($sites as $site) {
			if(eregi($site[0], $linkC)) {
			check(trim($linkC), $site[1]);
			$titleCode[] = $site[2];
			}
		}  	
		
		if(eregi('hotfile\.com\/dl\/', $linkC)) {
	    $hfFile[] = $linkC;
	    $titleCode[] = "HF";
	  }
	}
}

if($hfFile[0]) {
	$hflinks = array_chunk($hfFile, 500);
	foreach($hflinks as $hflink) {
		$hflink = implode("\n" , $hflink);
		$postfeilds = 'files='.$hflink;
		$pagel = curl('http://hotfile.com/checkfiles.html', $postfeilds);
		preg_match_all('/Existent/' , $pagel , $match);

		$hflink = str_replace("\n", '<br>', $hflink);
		if (count($match[0]) > 0) {
			echo 'alive: ';
		}else{
			echo 'down: ';
		}
		echo $hflink.'<br>'; flush();

	}
}
?>

Open in new window

0
Comment
Question by:XK8ER
5 Comments
 
LVL 2

Expert Comment

by:shdwmage
ID: 36500106
Other than separating the functions out to a functions.php type file, I can't think of a better way to do it.  Sometimes you just require a lot of fields and a lot of text, I wouldn't get to out of shape over it being long.  

I comment the heck out of all of my code, so it's always long anyhow.
0
 
LVL 35

Expert Comment

by:Terry Woods
ID: 36500871
Note that the eregi function is deprecated. You may like to proactively convert it to use preg_match unless you're planning on not upgrading PHP at all, ever. If you'd like help converting them, just ask.
0
 
LVL 108

Accepted Solution

by:
Ray Paseur earned 500 total points
ID: 36501992
From a computer science standpoint, making code shorter is never a reasonable objective.  Making it run faster, adding comments, upgrading to modern techniques (refactoring) are the kinds of things professionals might do.

On line 1 we have this: <? and that makes the code dependent on the "short tags" environmental variable.  Environmental dependencies are unavoidable, but are to be avoided anyway.  I would change that to <?php.  If you don't know why, try intermixing XML and PHP short tags to see the outcome.

On line 2 we have nothing, and I would add this: error_reporting(E_ALL);  It is more than a little helpful to see all the Notice and Deprecated messages.  For example, it can keep you from accidentally relying on an undefined variable.

On line 5 we have global $pagel;.  If you need a global value, it is almost always a better practice to put it into the $GLOBALS array.  Here is why that is a good idea.  Let's say there are two programmers working on the same software (and if the software is worth anything to the world there will be at least two programmers working on it!) and they must work in different offices.  The last thing you need is a variable name "collision" that causes the work of one to overwrite the variables of another.  If you inject a variable into the main scope of the script, you increase this risk.  If you use $GLOBALS instead, you create the ability to use a code scanner to look for instances of "$GLOBALS" (case-sensitive) and thereby avoid variable collisions.  The best-practices solution is probably to refactor the code into object-oriented programming, but simply getting rid of global declarations is a good start.

On line 12 we have curl_setopt($ch, CURLOPT_TIMEOUT, 20);.  Is 20 seconds realistic?  Most HTTP request complete in subsecond times.  I am very lazy and impatient.  I think I would not want to wait 20 seconds for an HTTP request to complete.

On line 24 we have return($pagel);.  See line 5.  This kind of programming evinces misunderstanding and confusion about the nature of variable scope, and that is a way of ensuring that catastrophe is not left to chance.
http://php.net/manual/en/language.variables.scope.php

On line 16-19 we have a conditional code block that is misaligned.  A good programming practice is to indent conditional code blocks, lining up the curly braces so the reader can immediately see what the heck is going on.  Personally, I like to use a modified Zend standard for indenting conditional code.  I place curly braces on their own line and indent the code inside 4 spaces.  Some text editors will automatically substitute a tab for 4 spaces.  Conditional misalignment is in evidence throughout the script.

On line 21 we have this: $pagel=curl_exec($ch);.  There is no error checking or visualization.  CURL is not a black box.  It can and will fail for reasons outside of your control.  So a reasonable expectation would be that a programmer would provide some kind of error handler.  You might want to learn about curl_errno() and curl_error().
http://php.net/manual/en/function.curl-error.php

Next, let's consider this construct:
$alllinks = '
http://www.megaupload.com/?d=UM3GFUZU
http://www.megaupload.com/?d=UM3GFUZZ
http://hotfile.com/dl/129240829/ad5cc51/Windows_7_ultimate_collection_of_wallpapers.74.jpg.html
http://hotfile.com/dl/129240829/ad5cc52/Windows_7_ultimate_collection_of_wallpapers.74.jpg.html
';

$alllinks = explode(" ", $alllinks);
$alllinks = implode("\n", $alllinks);
$alllinks = explode("\n", $alllinks);

Open in new window

It appears to be designed to create an array of URLs.  Maybe it could be written like this:
$alllinks = array
( 'http://www.megaupload.com/?d=UM3GFUZU'
, 'http://www.megaupload.com/?d=UM3GFUZZ'
, 'http://hotfile.com/dl/129240829/ad5cc51/Windows_7_ultimate_collection_of_wallpapers.74.jpg.html'
, 'http://hotfile.com/dl/129240829/ad5cc52/Windows_7_ultimate_collection_of_wallpapers.74.jpg.html'
)
;

Open in new window

All of those explode() and implode() statements do not add any value.

On line 52 we find this: "\n".  Like the short-tags issue, this creates an environmental dependency,  You might want to learn about the context-aware end of line constant, PHP_EOL.  It will make the code more portable.  This page is required reading for PHP programmers.
http://php.net/manual/en/reserved.constants.php

The code relies on variants of the deprecated ereg() functions.  You can either fix that or use an error reporting level with  (^DEPRECATED) to suppress the messages.  I would suggest you fix it - the fix is usually as simple as adding regex delimiters and flags for case-insensitivity.  Suppressing the messages is a little like putting a piece of black electrical tape over the "check engine" light on your dashboard.

That's as far as I got.  Without comments it's hard to follow the intent.  I usually write my comments first, then write my code.  Not only does this practice ensure that my code is well commented, it ensures that I can say in plain language what I want the program to do.  It's a tiny step in the overall process of consolidation of thought, but I have found that it serves me well to start with comments and follow with code, rather than the other way around.

HTH, ~Ray
0
 
LVL 2

Expert Comment

by:shdwmage
ID: 36502312
Ray I always enjoy reading your posts, they are so insightful. Quite often I learn a different way to do things in my own code based upon your insightful answers.

Thanks and to the OP he did a really good job of explain errors.  My original post intent was only about the length.  I agree with Ray that having no comments makes it difficult to figure out your code.
0
 
LVL 1

Author Comment

by:XK8ER
ID: 36504829
thanks so much, that was very helpful..
0

Featured Post

Better Security Awareness With Threat Intelligence

See how one of the leading financial services organizations uses Recorded Future as part of a holistic threat intelligence program to promote security awareness and proactively and efficiently identify threats.

Join & Write a Comment

Popularity Can Be Measured Sometimes we deal with questions of popularity, and we need a way to collect opinions from our clients.  This article shows a simple teaching example of how we might elect a favorite color by letting our clients vote for …
Foreword (July, 2015) Since I first wrote this article, years ago, a great many more people have begun using the internet.  They are coming online from every part of the globe, learning, reading, shopping and spending money at an ever-increasing ra…
The viewer will learn how to create and use a small PHP class to apply a watermark to an image. This video shows the viewer the setup for the PHP watermark as well as important coding language. Continue to Part 2 to learn the core code used in creat…
This tutorial will teach you the core code needed to finalize the addition of a watermark to your image. The viewer will use a small PHP class to learn and create a watermark.

707 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

13 Experts available now in Live!

Get 1:1 Help Now