The Daily WTF: Curious Perversions in Information Technology

2016-09-27 Reply Admin

First snippet - it may well be that "HR" needs to be singled out. But on Unixes readdir also returns "." (this dir) and ".." (parent dir) - these are also singled out. Which, from the rest of the code seems to be valid ....

2016-09-27 Reply Admin

Frist? Arrggggh

2016-09-27 Reply Admin

Normally if you need to exclude a folder you just set up a filter for that folder, instead of instituting a THOU SHALT NOT rule that'll inevitably catch some junior intern someday.

It'd be hilarious if HR was never supposed to be excluded, but because that's just the way the software worked, they worked around it by using other folders for their important files. Cargo cults are alive and well to this day.

2016-09-27 Reply Admin

Snippet 1 - Done badly? Yes. But it'll work, as Brian mentioned. There are better ways - pre-populate an array of values to ignore, but it'll still work for intended purpose. Snippet #2 is just someone that's gotten confused as to where to put the Throw. Not a mistake anyone above graduate level should ever make though. However, the last snippet is just tragic .

2016-09-27 Reply Admin

Snippet #2 does too do something! It destroys the stack trace! Obviously a feature!

2016-09-27 Reply Admin

That's why you test for the presence of a leading dot on the filename.

$imagename[0] != '.' && $imagename != 'HR' && ......

Much easier than just excluding everything with 2 or less characters.

2016-09-27 Reply Admin

Yes, I was thinking that too. I was also thinking, if the string comparison is slower than the string length, (a good optimizer might even make this check that the third character isn't null, if it is, then the string is not longer than 2 characters), the only WTF is that the reason isn't documented and instead the maintainer had to go research why that code was the way it was instead of seeing that in a comment.

Steve_The_Cynic · 2016-09-27 Reply Admin

"a good optimizer might even make this check that the third character isn't null,"

I sincerely hope not. What is the third character of "A" ? Or did I just whoosh again?

2016-09-27 Reply Admin

strlen($imagename)>2

it's clever than you think. It also excludes the . and .. directories

2016-09-27 Reply Admin

Test for the leading dot. Ah yes - because a directory named .settings (for instance) is obviously not a valid name.

Addendum 2016-09-27 08:32: apparently a LT slash s GT is stripped by the forum software. </s>

2016-09-27 Reply Admin

strlen($imagename)>2 also has the advantage that it avoids a file or directory named "0" stopping the loop, because obviously the string "0" is FALSE...

2016-09-27 Reply Admin

In this case, strlen() is slower than string comparison because one of the strings is immutable. At maximum, strcmp will do 2 char comparisons. The WTF is premature optimization.

2016-09-27 Reply Admin

"(a good optimizer might even make this check that the third character isn't null, if it is, then the string is not longer than 2 characters)"

And what if the string has length one, and the memory cell at string position + 2 is filled with some random bits?

2016-09-27 Reply Admin

PHP's internal string type stores the length, so there's no need to "look at every character in one string" because strlen() is O(1). Not really relevant in a loop that does disk IO, but it proves the PHP version of Muphry's law: every post or article bashing PHP contains at least one misconception about PHP.

2016-09-27 Reply Admin

It does not: the while condition is checked before strlen. The while should be written like this: while(($imagename = readdir($dir)) !== false )

2016-09-27 Reply Admin

Encountered this gem in a production environment a good while back (in BASIC);

i$ = i$

2016-09-27 Reply Admin

Of course, what was I thinking... My brain always starts to shut down when it tries to parse PHP

2016-09-27 Reply Admin

There could be a justification for the second snippet, depending on context. By constructing a new exception rather than rethrowing, it removes the stack trace. I've done something similar when dealing with security-related code where exceptions needed to be logged by the client code, but the stack trace might contain passwords for example.

2016-09-27 Reply Admin

TBH, when I look at #2, I just see someone planning to add some logging code, then never finishing. It's a little curious, but not that bad in the grand scheme of things.

2016-09-27 Reply Admin

If the stack trace has a password in it, you're doing it wrong.

2016-09-27 Reply Admin

#2 actually does something.

It mangles every class inheriting from Exception into a plain exception. This can actually make sense, if you e.g. have a call stack of the form A -> B -> A, whereby component A is unaware that it indirectly called itself, and a typed exception bubbling through B would be caught unexpectedly. Stripping the exception type at the component boundary can avoid that.

#3 is just a typo.

It should have been unset($product_data), which is needed as $product_data would otherwise pollute the parent scope.

2016-09-27 Reply Admin

Yeah, but what if my $IMPORTANT_DIRECTORY name is "K" ? what then?

2016-09-27 Reply Admin

All this proves is that in any given language, somebody will use it wrong, think they are using it right, understand their task incorrectly, fault the language, make fun of the language, and last but not least, make up a real 'WTF' mess for all of us to see,

It happens everywhere, and will keep on happening as long as there are programmers.

Then there are stories of ill guided management. Enough said.

2016-09-27 Reply Admin

Yes well in the last snippet there is a pathology where that does something. Consider:

https://github.com/leedavis81/vent

and suppose the $product_data array was set to fire events on read?

Yes it's utterly filthy...but then again, this is TDWTF!

2016-09-27 Reply Admin

I would put it inside another root folder that simply isn't indexed.

2016-09-27 Reply Admin

If you need to redact something for the stack trace, or remove the stack trace etc., then doing so in a throw/catch in each low-level method is the wrong place.

2016-09-27 Reply Admin

For added fun, $scratch_products might actually be a Iterator object that does all kinds of hidden stuff. It might even spew out ArrayAccess implementations, doing their own thing when offsetSet('productid') is called.

2016-09-27 Reply Admin

Not really. It depends on what the rest of the method does. It's not really a WTF without more context. Maybe it's a bad idea. Maybe it's not. We don't know.

Personally, I've done similar things before when I didn't think the full stack trace was necessary. Usually when interfacing with a third-party library. The basic thing i need is that "this" method is erroring in this location. I don't need 50 lines worth of trace from libraries i didn't write and can't modify.

Usually I'll modify the message as well, but sometimes it's easier or beneficial (generally when working with sql queries) to percolate the original message back up.

For example, if I'm using a third-party library that eventually hits an sql database, it's nice to get the -803 error back along with the knowledge that my attempt to call that third party library failed. But I don't need to know that my code called ThirdPartyLib->GetData() which called ThirdPartyLib->BuildQuery() which called ThirdPartyLib->SqlPrepper() which called ThirdPartyLib->SqlPrepper(string, string) which called ThirdPartyLib->SqlPrepperPreparedStatement(string, string, string), etc.

2016-09-27 Reply Admin

Nothing wrong with PHP, apart from the fact that it wasn't really designed as a language in the first place and has been mutated into something it was never originally intended to be (prior to version 3.0, at least). And nothing wrong with PHP programmers, except that they choose to program in PHP -- or at least get paid to do so, and nobody can argue with that. And nothing wrong with the rational position that I, amongst many others, hold: statistically speaking, PHP programmers are far more likely to be dangerous idiots than, say, Java programmers. (I am not keen on Java programmers either, but I am realistic.) And PHP programs are, statistically speaking, far more likely to be abject crap than, say, Perl programs (I have written some pretty dire Perl programs, so I'll hold my hand up.) And for anybody who repeats the meme "you can program badly in any programming language," well, yes. Equally, you can write a program that performs precisely to the customer's requirements in Brainf*ck. But who would want to?

2016-09-27 Reply Admin

The try-catch snippet is probably some kind of refactoring leftover. Like, you're enclosing a DB transaction within a try block and catch all exceptions just to roll the transaction back on any error, which is then re-thrown. Then at some point you get rid of the transaction (or move it elsewhere) and you're left with an idle try-catch-throw block... Sloppy? Yes. But really, WTF?

2016-09-27 Reply Admin

Then there are stories of ill guided management.

Sure, but this language even manages to misspell the third initial of that common abbreviation.

2016-09-27 Reply Admin

Encountered this gem in a production environment a good while back (in BASIC);

i$ = i$

Was that left over from a debugging breakpoint where the developer just wanted to glance at the value of i$?

At least I wouldn't have to resort to any similar stupidities when debugging C++ in Visual Studio. Or if I did, I would always remember to delete the line after finishing debugging. Or if I forgot, at least I'd catch it myself when reading my own code again a while later, and meanwhile the optimizer would optimize it out[*].

[* because release versions get optimized. We're not allowed to distribute debug versions, just like laws allow car manufacturers to install seat belts during testing but require seat belts to be removed when selling cars to the public. Or something like that.]

2016-09-27 Reply Admin

Php is a c-style language. All the sillyness of noobs. Write a proper unit test. Kthxbai.

2016-09-28 Reply Admin

"Nothing wrong with PHP..."

You should check this out, it's a good read.

https://eev.ee/blog/2012/04/09/php-a-fractal-of-bad-design/

2016-09-28 Reply Admin

Most interesting is, that in #3, although there is a foreach loop on $product_data, not the element $scratch_products is used but $product_data to access the field 'productid'. So the unset seems not to be the only problem here...

Steve_The_Cynic · 2016-09-28 Reply Admin

Sort of, except that the try/catch isn't idle. It catches all exceptions derived from Exception (which is presumably all of them, I'd hope, given that this isn't C++ where you can throw anything, even stupid things like int - throw 5; is valid C++...), junks them, and throws one itself. This discards the stacktrace of the exception location, and costs CPU and memory resources. I'd presume that you can just re-throw the current exception using a simple throw; statement, like in C++ and others.

2016-09-28 Reply Admin

...unless you want to catch config files that start with a dot...

2016-09-28 Reply Admin

"Most interesting is, that in #3, although there is a foreach loop on $product_data, not the element $scratch_products is used but $product_data to access the field 'productid'. "

It would help if you knew the language before making assumptions, speaking about it, and proving your ignorance. You're not the only one. Well over half of the preceding comments are clueless assumptions about the language as well.

The foreach loop is on $scratch_products, END OF STORY.

$scratch_products is an ARRAY (no chance it is anything else) END OF STORY.

$scratch_products has at least field, called 'productid'. END OF STORY.

$product_data is a local variable (within the scope of the loop ONLY) used as a pointer into the current row within $scratch_products

Substitute $foo for $product_data and the results are the same.

The ONLY WTF is destroying the results, a new array, $pids, containing ONLY productid in it, as soon as it was so carefully built.

I will guess WHY he did this. First envision $scratch_products is a query result (in PHP they're still just simple arrays) with 112 columns. productid is the 87th column. Something was going wrong with them and he wanted to see them only so he produced a mini-array of just the PIDs to see what the heck was going on (dunno where the echos went). He left the mini-array in place, destroying it rather than removing it. Yes silly indeed. Perhaps he wished to be able to turn it on again the next time something crashed. There are easier ways though, // comes to mind. At any rate, there is no WTF other than a few wasted CPU cycles. The program flow is totally unaffected.

PHP is a SIMPLE language for those who know how to and with a desire to keep things simple. Fields, arrays, manipulators, conditional flow, and IO. That's pretty much it, nothing more. WYSIWYG. As such it is also a beautiful language for programmers who enjoy the art of KISS.

The one thing it lacks is a Debug/IDE platform. There is no way to Debug Trace run time flow. If there's a problem, it's back to the old ways, drop in a display, Echo "I'm here and " . $foo . " just got changed again dammit "; Figure out the problem and yank it or comment the display. If you're doing displays to often you're not KISS'ing up to the language. Find another job, or learn how to KISS, you're choice.

2016-09-28 Reply Admin

It destroy also the type of the exception

2016-09-29 Reply Admin

Don't ever test strings using array syntax in PHP. That syntax treats the strings as arrays of bytes, not arrays of characters. It's a subtle but important difference. If you absolutely have to take the first character, then do it properly.

Medinoc · 2016-09-30 Reply Admin

@scragar and Brian Boorman:

I don't remember where I've read it, but apparently the whole UNIX "files starting with dot are hidden files" "feature" was actually created accidentally with such a test. The code for dir skipped files starting with a leading dot in order to exclude "." and "..", and someone noticed it worked on any file with a leading dot...

Addendum 2016-09-30 06:19: And by dir, I mean ls.

2016-09-30 Reply Admin

Php does have a really good IDE https://www.jetbrains.com/phpstorm/ with debugging tools a and a couple of different debuggers (https://xdebug.org/ mainly).

2016-09-30 Reply Admin

I find Snippet #3 to be... unsettling.

2016-09-30 Reply Admin

Conventionally, an object name beginning '.' is not listed anyway, so it could well be a valid design choice to skip all object names beginning with '.'. I've just done some such code earlier today.

2016-10-06 Reply Admin

Just to be clear - there's also a 'debunking' of this article, probably more than one. Like here: https://news.ycombinator.com/item?id=4177516

And yes, I'm PHP programmer (mostly), so my opinions are biased. But I really think, that with current frameworks (like, Symfony), good practices (whole PSR initiative), unit testing (PHPUnit) etc, not mentioning (well, mentioning actually) Travis and Scrutinizer support on GitHub, most of the new PHP code is of quite good quality.

BTW - I'm not arguing, that it's hard to write a bad code in PHP. No, it's easy. But I'd argue, that writing a bad code is also easy in most of the languages, including C.

Also, about the 2nd example. I'm actually doing something similar in my library. I implement kind of strategy pattern, where enclosed code can (should not, but can) throw arbitrary exceptions. I want to catch them all in my template function and convert to one type ProcessException. Something like that:

try {
    // Do something by external classes, which may behave bad
} catch (\Exception $e) {
    throw new ProcessException($e->getMessage(), $e->getCode(), $e);
}

Mind it, I'm maintaining reference to the previous exception, but 'unify' all into one type, so I can write in my class signature, which exceptions are thrown from it. I think that repackaging exceptions is not so bad and unique to PHP (if they were, Exception constructor in Java would not have the second argument).

2019-07-20 Reply Admin

` $dir = '/var/www/media/docs/';

$foldernames = array_diff(array_filter(scandir($dir), 'is_dir'), ['.', '..', 'HR']);

$nCount = count($foldernames); `

Properly Handled PHP

Leave a comment on “Properly Handled PHP”