• peter (unregistered)

    first? :)

  • first (unregistered)

    first

  • m (unregistered)

    3rd

  • (cs)

    Can I get some comment captioning for the PHP-impaired?

  • guru (unregistered)

    ...i don't get it

  • Vladimir (unregistered)

    A subject just missed ! in the first if.

  • Khomar (unregistered)

    Well, my PHP is rusty, but I would say the first biggie is trying to open a file that has already been determined not to exist.

  • Anonymous (unregistered)

    I haven't had any coffee yet, but it's first checking if a file exists, and then if the _exists check fails then it tries to read it in a loop.

  • Cyrus (unregistered)

    Going off my limited PHP knowledge.

    If the file exists it does what it was supposed to do with the file. If it doesn't not only does it do it anyway, but it goes into a while loop that will continuously generate errors but never leave the loop.

    Man oh man, WTF was this guy thinking...? Oh right, 'File not found eh? Well, open it anyway.'

  • Look at me! I'm on the internets! (unregistered)

    This code is just screaming for tri-valued booleans.

  • admin (unregistered)

    ...just looking for that nifty third boolean value.

  • Guest (unregistered)
    1. if (file_exists($file)) {
    2. $data = implode("", file($file));
    3. } else {
    4. $fp = fopen($file,'r');

    5. while(!feof($fp)) {

    6.    $data = $data . fread($fp, 1024);

    7. }

    8. fclose($fp);
    9. }

    1st line checks if file named (string)$file exists. If it exists - 2nd line reads file's data into a string $data. This is done by using funcion file(), which reads all file into an array - every sting in a file - an element in an array. After this reading this line implode()'s (joins) that array into string.

    If $file doean't exists the interesting part begins :) It still tries to open the file and read every line of it till it would reach the end of file. But what's the point of this else statemenet, if file doeasn't exists? WTF?

  • Khomar (unregistered)

    Ouch. As I look up the purpose of the functions, the two if clauses are virtually identical in what they do. The file() function reads the entire file into an array, and implode() takes all elements of an array and concatenates them together into a single string -- in this case, without any separating character. So basically, no matter if the file exists or not, the program will attempt to open the file and create a single string with the file's contents. The one that will actually work (the implode(file()) version), is the most inefficient way of the who.

    Next, even after the file has been determined not exist, as I said before, they try to open the file. After calling fopen(), they do not even bother to check to see if the returned value is null. The following loop is probably okay (if they file actually existed and opened successfully!), but in either case, if the file is very large, you could easily run into the resource errors mentioned. I wonder what they were actually trying to do with the file and what kind of files this code was expecting to open.

  • ep (unregistered)

    The following excerpt from the PHP manual might clarify things:

    PHP Manual on feof():
    Warning: If passed file pointer is not valid you may get an infinite loop, because EOF fails to return TRUE.

    Basically, the code checks if a file exists, and when it has ascertained the absence of the file, it tries to open it anyway in a way that the manual specifically warns against. The resulting error messages are happily mailed by cron.

  • Derek I (unregistered)

    To be more exact, fopen returns FALSE, a warning is issued, and feof(FALSE) is always FALSE, resulting in an infinite loop.

  • (cs)

    Is it just me or the code segments in the RSS feed look screwed up?

  • MCA (unregistered) in reply to Ezekiel Rage
    Ezekiel Rage:
    Is it just me or the code segments in the RSS feed look screwed up?

    I get that, too

  • (cs) in reply to MCA

    Ah but for a simple "!"...

    This required two simple unit tests to validate. sighs

  • The Wai (unregistered) in reply to Ezekiel Rage
    Ezekiel Rage:
    Is it just me or the code segments in the RSS feed look screwed up?

    It is just you brother Ezekiel.

  • (cs) in reply to Guest

    I see. So if the file exists, then read it. If the file does not exist, then read it slowly. --Rank

  • Jno (unregistered) in reply to Derek I
    Derek I:
    To be more exact, fopen returns FALSE, a warning is issued, and feof(FALSE) is always FALSE, resulting in an infinite loop.
    Well, that's it then. feof(FALSE) clearly *ought* to return TRUE!
  • Marcin (unregistered)

    Yes, this looks like a single character has been left off. As commented, simple unit testing would have found this problem. In the absence of unit testing, it illustrates the value of adopting certain code-writing practices, such as writing the test for the if-case without negation, because that's moderately hard to find by scanning, or commenting the test so that it's easier to verify.

    It's not as good as a unit test, but it does help with maintenance.

  • Amadan (unregistered)

    It's not the lack of "!". Both branches of the conditional attempt to do the same thing (i.e. read the file into the string). The WTFs are:

    • well, duh - both brances of the conditional do the same thing (by varying efficiency)
    • why read a file you know does not exist?
    • file_get_contents is a PHP function
  • Anaynous (unregistered)

    Actually, the else portion is an error notification. It just checks over and over to make sure it gets the administrator's attention.

  • Marcin (unregistered) in reply to Marcin

    Oh wait - I'm an idiot. It's not a missing !, as others have pointed out.

  • Anaynous (unregistered) in reply to Marcin
    Marcin:
    Oh wait - I'm an idiot.

    You said it, not me.

  • (cs) in reply to Rank Amateur
    Rank Amateur:
    I see. So if the file exists, then read it. If the file does not exist, then read it slowly. --Rank

    hehehe :)

  • freelancer (unregistered) in reply to Marcin
    Marcin:
    Oh wait - I'm an idiot. It's not a missing !, as others have pointed out.
    Nope, it's just missing a little thing called common sense :P
  • (cs)

    file_get_contents only exists in PHP 4.3 and later. That part is not a WTF if the script is older than that.

  • shortbread (unregistered)

    $data = $data . fread($fp, 1024);

    Should have been:

    $data .= fread($fp, 1024)

    but even then that's just evil. Looks like the $data buffer doubles in size + 1024 every iteration and STILL it's an infinite loop.

    captcha: dreadlocks (groovy!)

  • Noone (unregistered)

    The biggest WTF is that he sent PHP to do a job of a PERL, everyone knows that you shouldn't send a boy for a job of a man.

    (Yes I hate PHP, for valid reasons too)

  • (cs)

    Even if someone happened to create/copy/upload the file while it was infinite looping, it would continue to infinite loop. That is funny to me.

  • ep (unregistered) in reply to Jno
    Jno:
    Derek I:
    To be more exact, fopen returns FALSE, a warning is issued, and feof(FALSE) is always FALSE, resulting in an infinite loop.
    Well, that's it then. feof(FALSE) clearly *ought* to return TRUE!

    The PHP api is truly a frightening minefield of inconsistencies.

    I once did a very nice online quiz about the murkier parts of PHP's api. It had multiple choice questions like 'what does is_empty(null) return?'. You could also get statistics about the received answers. The median of correct answers was about 80%. I personally landed 92%, and this was after 5 years of intensive work with the language. Can't seem to locate the quiz now, perhaps someone else knows where it lurks?

    Anyway, several years of PHP has made me turn to cargo cult programming to preserve my sanity. I do feel occasional pangs of guilt about it, though.

  • dirkie (unregistered)

    Maybe he meant

    if (function_exists("file")) {
  • jlh (unregistered) in reply to Noone
    Noone:
    The biggest WTF is that he sent PHP to do a job of a PERL
    I'm sure the author of that code snippet could easily messed up the same thing in perl too.
    Noone:
    everyone knows that you shouldn't send a boy for a job of a man
    Yes, you're allowed to have your opinion. :) Other people's opinion may differ.
  • SomeCoder (unregistered) in reply to Noone
    Noone:
    The biggest WTF is that he sent PHP to do a job of a PERL, everyone knows that you shouldn't send a boy for a job of a man.

    (Yes I hate PHP, for valid reasons too)

    I actually love PHP but this statement is all too true in my experience... I know a PHP programmer who will write all his cron jobs/tasks/whatever in PHP. The jobs are usually WAY more suitable for Perl but he insists on using the command line PHP for it.

    The right tool for the right job does not mean that you should use a web programming language for scheduled/cron tasks....

    Captcha: cognac.... a little early but if you insist!

  • (cs)

    I am reminded of a piece of dialogue from an episode of Dexter's Lab called Ocean Commotion:

    Captain: Arrr ... the great big crap-like thing ... I've bin searching all me life fer it ... First Mate: Uh, excuse me captain, no you haven't. Captain: Arr - harpoon it anyway!

  • chrxs (unregistered)

    Seems like the project manager from Ability to Bend Space-Time a Must went on to become a php programmer. It's the same logic here: If (no survey results yet) then (give me the survey results anyway)

  • Late today (unregistered) in reply to Rank Amateur
    Rank Amateur:
    I see. So if the file exists, then read it. If the file does not exist, then read it slowly. --Rank

    Hmmm - when we Americans encounter someone who doesn't speak English, don't we try to speak more slowly, as though that will make the other person understand us?

    Apparently, this person felt the same way about his code to read a potentially missing file...

    BTW: for all who posted that it's missing a "!" (myself included); it was understood (!) that it was supposed to be something like:

    if (!file_exists(...)) {
   handle missing file
} else {
   read it slowly
}

    which we can all agree is arguably still as stupid as compared with just reading it in one clump.

  • Peter Antoine (unregistered) in reply to Rank Amateur
    Rank Amateur:
    I see. So if the file exists, then read it. If the file does not exist, then read it slowly. --Rank

    "Englishmen abroad" programming.

    i.e. If they don't understand what you said in English the first time, say it again louder and slower. They will understand then. :-)

  • (cs) in reply to guru
    guru:
    ...i don't get it

    If it exists, read it.

    If it doesn't exist, open it and then read it.

    Sort of like this:

    if ($i!=0) print 5/$i;
else print 5 * $i^-1

    Makes sense, yes?

  • tux_rocker (unregistered)

    It seems like the author assumed that if the file does not exist on the local filesystem, then it must be a URL. And under certain PHP configurations, fopen() can open remote files if you pass it a URL. file_exists() however only checks for local files.

  • (cs) in reply to shortbread

    It looks like the author of the snipped was very committed to the idea of not using file() if file_exists() fails. This is generally a good practice, described in the manual. [S]he just missed the else part...

    shortbread:
    $data = $data . fread($fp, 1024);

    Looks like the $data buffer doubles in size + 1024 every iteration and STILL it's an infinite loop.

    No I doesn't. It's not $data .= $data . fread($fp, 1024);

  • RJ (unregistered) in reply to Noone
    Noone:
    The biggest WTF is that he sent PHP to do a job of a PERL, everyone knows that you shouldn't send a boy for a job of a man.

    (Yes I hate PHP, for valid reasons too)

    There are plenty of reasons to hate PHP, but in general, there is a fairly fixed number of WTFs and once you know them, it's not so bad. Perl, on the other hand is one giant WTF of arcanity and outright wierdness.

  • Daniel (unregistered)

    The real WTF here is that PHP actually reads something from that not existing file.

  • ep (unregistered) in reply to tux_rocker
    tux_rocker:
    It seems like the author assumed that if the file does not exist on the local filesystem, then it must be a URL. And under certain PHP configurations, fopen() can open remote files if you pass it a URL. file_exists() however only checks for local files.

    Good point.

    Then again, URL wrappers work with file(), so he could have just used that anyway. And as of PHP 5, file_exists() does actually check over the net for some protocols.

  • (cs)

    Remember folks, the acronym for PHP Object Oriented Programming is "POOP".

  • cm (unregistered) in reply to ep

    Well, actually "is_empty(null)" prints out Fatal error: Call to undefined function is_empty()

    The function you meant is called empty() ;-) I mix them up all the time, too ...

    One of the biggest WTFs in PHP itself is the fact that a string consisting of a single character zero (the digit zero!) is considered to be empty.

  • Philihp (unregistered)

    Some languages support both the "not" keyword and the "!" operator. It's instances like this that I prefer spell it out.

    if( not file_exists(foo) ) {

    “Programs must be written for people to read, and only incidentally for machines to execute.” – H. Abelson and G. Sussman (in “The Structure and Interpretation of Computer Programs“)

  • (cs) in reply to freelancer
    freelancer:
    Nope, it's just missing a little thing called common sense :P
    This site keeps convincing me common sense is not that common

    Addendum (2007-01-29 11:44):

    dirkie:
    Maybe he meant if (function_exists("file")) {
    Good point (bad typo, the keys are just next to each other) A good WTF gives a good set of semi-sane answers to the question "what's actually intended"...

Leave a comment on “Well, Open it Anyway”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article