Comment On Authenticate or Math

As others have said, the || clause is probably left from debug use.

As to why 2+2==4 rather than 1 or True, none of you seem to allow for humour. No doubt the programmer found it amusing to write it that way.

Jason:

It's obviously debug code that either the original developer forgot to take out, or was purposely left in to bypass having to constantly log in. Since it was an unfinished project it's likely the latter and the new guy needs to get off his high horse. In my experience many developers when having to take over a new project will trash the work of the previous generation since it's easier to do than actually having to really learn the architecture.

Egzackery....We have a captcha that requires some math, but the devs got sick of entering their details AND the captcha in the dev sandpit so they temproarily hacked it.

Then the testers wanted same in SIT...

Not really sure how it made production though. Never mind, the users don't seemed to have worked out they can put any value in our math captcha

There is also a chance that the getter for the logged_in boolean is overloaded to actually to log in the account if it is not.
It could also log this activity.

This means that if an account has its details available but is not logged on or if it is logged on, it will write to the log.

It will let you upload even if it can't...

Pinkie Pie:

lol omigosh frist!!11!!

And Akismet thinks this is fine??? WTF?

Leo:

No good, because what if 2 changes so that 2+2 no longer equals 4? Should be "|| 2+2 == 2+2", so even if 2+2 = 6, it will still evaluate correctly.

What if the value changes between the evaluation of the left and right hand operands? You're setting yourself up for a race condition there!

wbrianwhite:

It's clearly debug code. I on the other hand actually have "where 1 = 1" code in production. And I don't consider it a WTF. When appending various conditions to dynamic sql it's easier to start with a no-op condition and then append all the other conditions starting with "AND" without keeping track of "is this my first condition? no? then throw in and".

I hate lazy coders.

After taking down three commenters in quick succession, I'll stop now.

Peace.

da Doctah:

We all realize, of course, that logged_in() has side effects, and this is a way to ensure that those side effects take place while in effect throwing away the result of the check?

Too bad function invocation isn’t supported outside conditionals, eh?

2+2=5
For sufficiently large values of 2
FTFY

Edward:

Canonymous Oward:

Actually, the code might have a pretty legit reason. In some cases you can not just put "true" into "if" condition if there is "else" branch in the code, the compiler will complain about unreachable code.

Seeing this in the production code kind of sucks though.

There's a good reason why it would complain of unreachable code.

Because it is.

Which I do not really care about when I'm debugging the code this way. However, I do want the compiler to check that this code is compilable, so I'm not ok with just commenting it out.

The code is wrong though - it can be reduced by compiler to a constant. The Right Way (tm) in Java is to put something like "".equals("")

First rule of bad code: never assume a method is without side-effects!
Maybe $this->ion_auto->logged_in() also parses the cookie and writes a global $user variable somewhere, which the $this->load->view('') method depends on?

Captcha: 'mara', ancient old Buddhist mantra meaning 'nothing is without consequence'

ubersoldat:

I feel obligated to write this since no one has done it yet. TRWTF is PHP!

Actually, TRWTF is PHP syntax... who thought about using -> for object access? Really, what's the explanation for using TWO signs (which in es_ES keyboards takes THREE key-strokes) when a dot works perfectly fine in almost every other language in the world?

If you wanted to fuck up the syntax, why not use \_> which takes 6 key-strokes?

yeah thanks for pointing out php sucks again ... that's a really helpful and amusing point

RogL:

Could make sense as a temporary debugging change, to force the login to work while testing.

A distinctive "true" value is easier to remove when testing is done.

When it's time to remove the bypass, which would you rather search for: "true" or " || 2+2=4" ?
You probably don't want to blindly remove all "true" strings but you can safely search&replace " || 2+2=4" with "".

Good call.

But in that case what ought to have been programmed was for the "or" condition to be something like "|| OverrideLogin" which would then be assigned the value "True" somewhere at the start / compile time / whatever level of control you'd need.

If in Java then you might use the technique of calling it OVERRIDE_LOGIN and declaring it as a public static final boolean.

You C-monkeys would probably use #Define in a precompiler.

And a really good compiler would optimize the whole check out, since it can prove that the right-hand side is always true, so the if condition is always true, regardless of what is returned by the left-hand side.

Right??

Jason:

It's obviously debug code that either the original developer forgot to take out, or was purposely left in to bypass having to constantly log in. Since it was an unfinished project it's likely the latter and the new guy needs to get off his high horse. In my experience many developers when having to take over a new project will trash the work of the previous generation since it's easier to do than actually having to really learn the architecture.

very 2+2=4

jim:

KattMan:

So realyl, it doesn't matter if you are logged in.
The || essentially makign it so if you are logged in you can upload, if you are not logged in, you can upload, because in our reality 2+2 does equal 4.
Why not do away with the check in it's entirety? You will get the same results.

You don't say.

So wait, why did the chicken cross the road again?

As others have noted, this is pretty obviously debugging code. He did say it was an unfinished project.

I think I'll submit an hysterically funny WTF along the lines of:

I found this code in a project under development:

public BigDecimal calcSalesTax(int receiptNumber)
{
// TO DO: Get sales tax calculation in here
// For now just return a dummy value
return new BigDecimal("1.00");
}

Ha ha! The programmer never really calculates the sales tax! What a moron!

DEEmery:

And a really good compiler would optimize the whole check out, since it can prove that the right-hand side is always true, so the if condition is always true, regardless of what is returned by the left-hand side.

Right??

Depends if the compiler is smart enough to figure out that the function has no side effects. "Optimizing away" functions with side effects is not an entirely good thing.

Dirk:

Leo:

No good, because what if 2 changes so that 2+2 no longer equals 4? Should be "|| 2+2 == 2+2", so even if 2+2 = 6, it will still evaluate correctly.

What if the value changes between the evaluation of the left and right hand operands? You're setting yourself up for a race condition there!

Or what if the reflexive property of equality ceases to hold? The problem is bigger than you think.

fishdude:

wbrianwhite:

And I don't consider it a WTF. When appending various conditions to dynamic sql it's easier to start with a no-op condition and then append all the other conditions starting with "AND" without keeping track of "is this my first condition? no? then throw in and".

Since you are dynamically building an SQL statement, I'll assume you are using PHP.

Put all your WHERE clauses into an array, then use `implode()` to join the arrays into a string.

$where[] = "param1 = 'fish'";
$where[] = "param2 = 'slap'";
$sql = "SELECT * FROM table WHERE " . implode(" AND ", $where);

DotNet has a similar feature, string.Join(" and ", conditionArray), but what if none of the conditions need to be added? His code still works, yours doesn't.

Yeah ^^ TRWTF is posting this article in the first place... it really just amounts to "oh look, someone forgot to take out debug code (and the other code sucked too, trust me)". Takes a shitty coder, and a shitty person, to get excited about that IMO.

RogL:

Could make sense as a temporary debugging change, to force the login to work while testing.

A distinctive "true" value is easier to remove when testing is done.

When it's time to remove the bypass, which would you rather search for: "true" or " || 2+2=4" ?
You probably don't want to blindly remove all "true" strings but you can safely search&replace " || 2+2=4" with "".

I always replaced it with /**/... in case I ever had to go back (i'm alway paranoid about that and sometimes it pays off). eg replace " || 2+2==4" with "/* || 2+2==4*/" so that it would still be there but it wouldn't evaluate at compile time.

I've also been told that i'm wierd.

Medinoc:

Looks like an "always true" clause added in the condition to force the behavior, probably temporarily for debugging purposes.

TRWTF is the lack of comment/TODO/etc. about it and the fact it was checked in.

Exactly.. this is not a WTF, it's just an example of someone forgetting a step, quite possibly because he or she was busy working bug reports / change requests.

dkf:

veggen:

Do I get extra credit for recognizing CodeIgniter? ... No? ... Ok...

But you do get credit towards your next visit to the psychotherapist.

Psycho the rapist? That's how your mom calls me.

As far as the atrocious standard of 99% of all PHP code goes, this is actually pretty good. The original developer at least had some understanding of functions and OO, two concepts that are normally harder to convey to PHP programmers than teaching your dog calculus.

What makes you think they're using source control?

PiisAWheeL:

RogL:

Could make sense as a temporary debugging change, to force the login to work while testing.

A distinctive "true" value is easier to remove when testing is done.

When it's time to remove the bypass, which would you rather search for: "true" or " || 2+2=4" ?
You probably don't want to blindly remove all "true" strings but you can safely search&replace " || 2+2=4" with "".

I always replaced it with /**/... in case I ever had to go back (i'm alway paranoid about that and sometimes it pays off). eg replace " || 2+2==4" with "/* || 2+2==4*/" so that it would still be there but it wouldn't evaluate at compile time.

I've also been told that i'm wierd.

Around these parts, we have a function called DevMode() which returns true when it detects that it's running on the dev server.

So it would look something like:

if ($this->logged_In() || Devmode())
...

Devmode() returns false in production environments, true on dev/test environments.

Obvious leftover 'force this code path' hackery.

I myself am fond of temporarily removing code with

#if 6 == 9
...
#endif

http://en.wikipedia.org/wiki/If_6_Was_9

Your logic is flawless in any universe.