Yep, I think the same if the superficial similarity to Perl's exists query implies the same consequences in PHP. In Perl

exists($someHash{$keyA}{$keyB})

would end up creating and undef entry $someHash{$keyA} and then try to dereference the undef as a hash, resulting in an error. Not really a fix but perhaps someone with more knowledge of PHP can enlighten the rest of us here.

God. Why? I've been doing this 16 years now and I've had this crap bite me in the ass a few times. I'd take code that I could not read and fixed it like this just to break some fringe case. LEAVE IT ALONE!

In any sane language you don't need any parentheses to simply read and understand what should be going on. :P If the first statement evaluates to true, why should you care what happens in the "else" case? That's right, you shouldn't.

That's a fantastic way for the entire code base to rot into an un-maintainable mess. "No one can parse out what it's trying to say, but it's generating the right output for now, so let's never, ever touch it." This is a great way to trap yourself in a tangled web of unreadable garbage.

Messy code is broken. Obviously you don't want to make reckless or unnecessary changes, but properly defining your requirements and documenting and testing your code are all there to allow you to make changes when you need to.

Regardless of what your particular Software-cum-psychiatrist labels as a sane vs. an insane language, you should ALWAYS ALWAYS use parentheses. For one thing, it's even better than proper commenting as a tool for making your code comprehensible to a wide audience, not just the anointed few who are experts in the chosen language.
For another, one day soon you will be a little sleepy and write something with the wrong default precedence and spend the next 2 weeks tearing your hair out trying to find the bug.
Use parentheses. It doesn't cost anything at runtime.

True (although only true for nested ternaries) because PHP takes the low ground on operator associativity in this case and is basically the only language that gets the associativity the wrong way round. I can't guarantee that it hasn't been fixed since PHP5 or so, but I'd doubt it, because once you mandate an associativity, you're pretty much stuck with it.

False, because without knowing more details, we don't actually care about the associativity. Either the original code worked, or it didn't. Well, I say "we don't care," but actually we do, so the apparently rational refactoring might actually change behavior precisely because of PHP's broken associativity.

Also false, because the refactoring is broken, as the earlier commenters point out.

Really, the only reason to object to the original code (provided your mandate is to write PHP code) is that the indentation is completely obfuscated. Fix that so that it more closely resembles an if-else construct, which after all is what a ternary operator does, and you're copacetic.

If I were this young man's boss, I would tell him to revert the change, apply only whatever business requirement was on the ticket, ... which appears to be what these supposedly evil bosses did .... and to go away and write a whole bunch of unit tests. Because unless you have a reasonable degree of confidence that fiddling around with tiny largely unimportant details won't change the logic, then you simply do not fiddle around with tiny largely unimportant details.

Not the most impressive submission we've seen, if I may summarize the general opinion of commenters so far.

"Drive By Coding" is evil. The change should have been rejected as part of an unrelated ticket. THat being said, there should be a light weight means of producing the necessary "ticket" so that the code could be changed [and thus validated independently]

The submitter clearly showed their naivete here, for making this change without a ticket assigned. He should have related it to the ticket he was currently working on, and then none would be the wiser.

Just a note... the new code has a bug in it, PHP's isset provides protection and checking only for the last level of indirection reduced. An acceptable solution might have been $categories = @$categoryMap[$department][$classification]; but where I've been we like to avoid relying on the silence operator, so either write your own recursive isset checker or do each one by hand (unless, of course, code not seen here is making it impossible for that first level to not exist.

For what little PHP I know, I checked isset and the new version of code looks legit and should behave the same as the prior version. Unless I am missing something here? PHP stops execution at the first case that it can't find something at a level in the multi-dimensional array and returns false.

Regardless, all this hate for ternary operators and this looks just fine to me... Again, I'm not a regular PHP dev so sorry if I eff'd up some syntax:

... if you are a wimp who is too afraid of being yelled at to do your job.

I've had this crap bite me in the ass a few times.

I'm one of the "old guys" who simply toughened up and isn't afraid of being bit.

If you do your job and can show that you had a good reason for the changes you make and that you exercised reasonable precautions, you won't get fired. Moreover, you won't be perpetually afraid of doing your job.

Assuming your "$department" is meant to be "$product['department']", etc., then the new code is just as valid as the original - neither check that $product['department'] and $product['classification'] both exist.

While '@' will suppress the error report, error handlers will still run, so it's best avoided if feasible.

I've done this a few times when I brain farted and forgot how isset works which is a bit strange as a language construct, although a ternary is madness. It's common to also overlook that isset returns false for entries that are set but set to null. It gets mildly weirder in scenarios where given [a, b, c], a or a, b must be set but not c.

If you turn notices, errors, warnings on in PHP, the error reporting to full that is, then convert all errors into exceptions then that captures a hell of a lot of bugs. It's a bit like use strict in JS. This also means putting explicit checks in a lot of places to avoid notices.

Unfortunately the language isn't quite written to support that fully and in a few areas you have to put in kludges. The null coalescing operator improves things.

PHP has loads of internal error checking that you don't need to do yourself on things like IO where in most cases you simply want everything to fail/stop on an error. Most checks in real life are sanity checks for errors you can't really very gracefully handle in the local or a near scope. You still have cases with things such as when using socket, where states can be represented by errors. Then you have to use the @ operator to suppress the error and handle it yourself.

As PHP is a dynamic language (which means you save a huge number of lines of code with runtime polymorphism). This makes it important to have or turn on those checks. Otherwise it's like compiling a complex C program with the minimum warning level, etc.

It wouldn't surprise me if the original code has some values instead of just nulls for the 2 fail cases, in which case it would make a ton of sense. Regardless don't change working code just because you find the syntax hard to read.

TRWTF is the "cleverness" of the ternary, with all the language-specific side-effects et al.
Apparently no one has read: https://www.simplethread.com/dont-be-clever/ or the multitude of good coding practices that practically scream the same.

Good code is self-documenting. There is NOTHING self-documenting about that ternary horror.

Unless you're writing real-time code, you always can and should make it more reader-friendly. Because code sticks around longer than you.

" It's common to also overlook that isset returns false for entries that are set but set to null."
It's easy to avoid this mistake if you regard null as indicating the absence of a value. Then, isset is a test to see if an entry (an array element in this case) has a value (i.e., "is set") or not.

Thanks to the runtime polymorphism you mention, an entry can fail to have a value for two reasons: it might be null, or it might not exist at all. isset() is written to cover both cases, while other constructs exist to provide more refinement if it's needed.

It might be argued that the two cases should be semantically identical: either "absent entries evaluate to null" or "null assignment destroys lvalues". The first would silently swallow errors due to accidentally using the wrong array keys or object properties; the second would be a shotgun blast to the symbol table: anything that MIGHT be null would have to be explicitly checked before use, and code branched to handle both cases (no, you can't just assign a default a value and proceed - the whole point of nulls is that might not be any appropriate value to use).

What PHP did was "absent entries evaluate to null, but you're notified when it happens because you're probably doing something wrong".

'Course, then you get people writing isset($localvariable) because they can't work out if they initialised the variable or not.

I've also worked at places where code remained un-refactored because there was now ticket to fix it.

One of the reasons for this was that if you refactored something, you needed to prove that you didn't break anything. Proving you didn't break anything required that QA test it. Getting QA to test it required a ticket that could be signed in triplicate, sent in, sent back, queried, lost, found and then promoted to the QA server.

The red tape could've been cut with unit tests, but naturally we didn't have those either.

The most amusing thing about this thread is that someone used the behavior of Perl to model PHP behavior. PHP was invented specifically to be "Perl but somewhat sane." While there's still a couple of Perlisms in PHP, almost all of it has been relegated to the sanitarium to play checkers with Ada and IPL.