feedblitz counter output function

Coding Critique is the place to post source code for peer review by other members of DevNetwork. Any kind of code can be posted. Code posted does not have to be limited to PHP. All members are invited to contribute constructive criticism with the goal of improving the code. Posted code should include some background information about it and what areas you specifically would like help with.

Popular code excerpts may be moved to "Code Snippets" by the moderators.

Moderator: General Moderators

Post Reply
User avatar
psurrena
Forum Contributor
Posts: 355
Joined: Thu Nov 10, 2005 12:31 pm
Location: Broolyn, NY

feedblitz counter output function

Post by psurrena »

Here's a basic function to output the subscriber count. Anything missing or unnecessary?

Code: Select all

<?php
	//Output Feedblitz Counter
	function openStats($link,$mode=NULL){
		$m = (empty($mode)) ? $mode : 'r';
		
		$f=file_get_contents($link,$m);
		
		$stats = (empty($f)) ? 'There was an error...' : $f;
		
		return $stats;
	}
?>


<?=openStats("http://assets.feedblitz.com/chicklets/email/i/2d/XXXXX.txt"); ?>
User avatar
Darhazer
DevNet Resident
Posts: 1011
Joined: Thu May 14, 2009 3:00 pm
Location: HellCity, Bulgaria

Re: feedblitz counter output function

Post by Darhazer »

empty will return true for 0 ;)
your function relies on allow_url_fopen setting
The mode param is wrong, as you are not using fopen, and the second argument to file_get_contents is a boolean value - bool $use_include_path
enough?
User avatar
Mordred
DevNet Resident
Posts: 1579
Joined: Sun Sep 03, 2006 5:19 am
Location: Sofia, Bulgaria

Re: feedblitz counter output function

Post by Mordred »

- short tags
- return value is a string
User avatar
psurrena
Forum Contributor
Posts: 355
Joined: Thu Nov 10, 2005 12:31 pm
Location: Broolyn, NY

Re: feedblitz counter output function

Post by psurrena »

Like so? Not sure what is meant by short tags / return value is a string...

Code: Select all

<?php
	//Output Feedblitz Counter
	function openStats($link){	
		
		$f=file_get_contents($link,use_include_path);
		
		$stats=(empty($f)) ? 'There was an error...' : $f;
		
		return $stats;
	}
?>

<?=openStats("http://assets.feedblitz.com/chicklets/email/i/2d/22543.txt"); ?>
User avatar
Mordred
DevNet Resident
Posts: 1579
Joined: Sun Sep 03, 2006 5:19 am
Location: Sofia, Bulgaria

Re: feedblitz counter output function

Post by Mordred »

Code: Select all

<?php
        //Output Feedblitz Counter
        function openStats($link){      
                
                $f=file_get_contents($link,use_include_path);
                
                $stats=(empty($f)) ? 'There was an error...' : $f;
                
                return (int)$stats; //force as integer.
        }
?>

<?php echo openStats("http://assets.feedblitz.com/chicklets/email/i/2d/22543.txt"); ?>
Actually the last is a matter of style, so feel free to ignore it.
User avatar
psurrena
Forum Contributor
Posts: 355
Joined: Thu Nov 10, 2005 12:31 pm
Location: Broolyn, NY

Re: feedblitz counter output function

Post by psurrena »

Great, thanks!
User avatar
Darhazer
DevNet Resident
Posts: 1011
Joined: Thu May 14, 2009 3:00 pm
Location: HellCity, Bulgaria

Re: feedblitz counter output function

Post by Darhazer »

Code: Select all

 $stats=(empty($f)) ? 'There was an error...' : $f;
return (int)$stats;
WTF? (assign a string and cast to integer)
why not

Code: Select all

function openStats($link){      
                return (int) file_get_contents($link);
        }
User avatar
Mordred
DevNet Resident
Posts: 1579
Joined: Sun Sep 03, 2006 5:19 am
Location: Sofia, Bulgaria

Re: feedblitz counter output function

Post by Mordred »

Err well ... that's what happens when you illustrate a point without looking at the rest of the "points" around ;)
You're right of course.
max529
Forum Commoner
Posts: 50
Joined: Sat May 19, 2007 4:10 am

Re: feedblitz counter output function

Post by max529 »

Hi,

I would write the function as below

Code: Select all

<?php
        //Output Feedblitz Counter
        function openStats($link,$mode=NULL){
                $m = (is_null($mode)) ? $mode : 'r';
               
                $f=file_get_contents($link,$m);
               
                $stats = (strlen($f)==0) ? 'There was an error...' : $f;
               
                return $stats;
        }
?>


<?=openStats("http://assets.feedblitz.com/chicklets/email/i/2d/XXXXX.txt"); ?>
why is_null ?

improves readability. Behavior of empty function is not as apparent as behavior of is_null function. for example empty("0") return true.

why strlen?

Strictly speaking, if that file contains the string "0" your code will echo the 'There was an error...' message, because "0" is considered to be empty.
Strlen returns 0 for both the empty string and the boolean value false, the two possible return values of `file_get_contents` you would like to echo an error message.

hope this doesnt sound stupid.....

Regards,
Sandeep/Max
Post Reply