Comment On The Van Gogh Awards

OMGWTFBBQ?

 

captcha: zork 

Anonymous:

OMGWTFBBQ?

OMGWTFBBQ.

With out a doubt.

And to think, my Intro to Programming Environments 152 lecturer said "Never worry about making efficient code - That's what optimizers are for!"

The best part was that they bother to QA code to check for these sort of things.  It's so anti-WTF, this WTF doesn't deserve to be a WTF.

TRWTF is that there's actually a company which actively finds and sorts out WTFs.

Doing years, quarters, minutes, seconds but not hours... that made me laugh out loud

 

Tim Gallagher:

Note: Revealing class names have been replaced with xxx's -- the original developer may have made some mistakes, but, at least in this case, really bad names was not one of them.

Yeah, because nothing says great class name like "enmCCLAppState".

Tim Gallagher:

     System.TimeSpan TS = new System.TimeSpan(startDate.Ticks-endDate.Ticks);

Looks buggy to me as well. Doesn't this create a negative time span?

 

Apart from issues already mentioned, what I really hate about this
code is that it offers the opportunity to provide random strings for
HowToCompare, and then doesn't tell you if you got it wrong; instead it
just returns some answer that may or may not have any relationship to
the value you wanted.

Why not use an enum? And why not stick an "assert (false)" in that default clause?

I really enjoy this : 

if (p.GetType().ToString() == "Cxxxx.Cxxxx.Cxxxx.Wxxxx.AxxxxMxxxxBxxxx")

Anyone for a "class renaming" party ??
 

Tim Gallagher:

if (p is AxxxxMxxxxBxxxx)  {    _appState = (enmCCLAppState)((AxxxxMxxxxBxxxx)p).AppStateId;  }

This must be the real WTF!

Avoid double dynamic casts with:
AxxxxMxxxxBxxxx foo = p as AxxxxMxxxxBxxxx;
if(foo != null) _appState = (enmCCLAppState)foo.AppStateId;

Do note that if type information is not available at compile time, this is exactly what you have to do.

Or unavailable at runtime, as is the case if you've modules that can be plugged in at deployment. But, then the developer should have commented this to make the intention clear.

 // lets try some reflection so I feel like I'm learnng something new today, and can convince myself I'm a real smartypants.
// I mean, why walk when you can double back flip?

 captcha: creative

Tim Gallagher:

if (p.GetType().ToString() ==

"Cxxxx.Cxxxx.Cxxxx.Wxxxx.AxxxxMxxxxBxxxx")

if (props[i].Name == "AppStateId")

Wow, if this is supposed to be Java code, then String comparisions should not be made via "==" but via the equal() function.

No way for it to work indeed.

Anonymous:

Tim Gallagher:

if (p.GetType().ToString() ==
"Cxxxx.Cxxxx.Cxxxx.Wxxxx.AxxxxMxxxxBxxxx")

if (props[i].Name == "AppStateId")

Wow, if this is supposed to be Java code, then String comparisions should not be made via "==" but via the equal() function.

No way for it to work indeed.

Anonymous:

Wow, if this is supposed to be Java code, then String comparisions should not be made via "==" but via the equal() function.

No way for it to work indeed.

Yeah, and if it's supposed to be Lisp, they got all the parentheses in the wrong places.  If it's supposed to be Perl, they forgot all the $ and @ signs.  If it's supposed to be Haskell, they shouldn't be trying to use assignment statements.... etc, etc...

Anonymous:

Tim Gallagher:

if (p.GetType().ToString() ==
"Cxxxx.Cxxxx.Cxxxx.Wxxxx.AxxxxMxxxxBxxxx")

if (props[i].Name == "AppStateId")

Wow, if this is supposed to be Java code, then String comparisions should not be made via "==" but via the equal() function.

No way for it to work indeed.

Tim Gallagher:

private double DateDiff(string howtocompare, System.DateTime startDate, System.DateTime endDate)  {
   double diff=0;
   try
   {
     System.TimeSpan TS = new System.TimeSpan(startDate.Ticks-endDate.Ticks);

     #region converstion options
     switch (howtocompare.ToLower())
     { 

< snip >

I have seen this code in a production system before. A little Google finds (among others)

http://www.aspfree.com/c/a/ASP.NET/DateDiff-function-in-C-to-simulate-VBScripts-by-Robert-Chartier/

I think someone liked the VB style DateDiff() and other just copied it in...

 

  -Edoode

 

 

sweet.  this is like putting a "Windows Timer" control upon every page and using it to determine when a button is enabled/disabled (such as login, view disclaimer, etc.) as some programmers/idjits want *total* control over what happens in their app.

<captcha: perfect splendid excellant YOUR still and geek go code something>

He may have told you that instead one should focus on simple code instead. If that gives speed gainst that is a nice extra. The most important thing in writing code is to make sure it is understandable by a reader. (Note that you too will have forgoten the code when asked to look at it half a year from now).

This whole CodeSOD is a waste of space. They are BORING!

Anonymous:

This whole CodeSOD is a waste of space. They are BORING!

Shut up before we're back to 1 post a day again.  Even boring WTF's are good at wasting time.

The last one was hard to read, here's an easier version:

 

    if (p.GetType().ToString() == "com.company.product.ABCWidget") {

       Type myType = p.GetType();
       PropertyInfo[] props = myType.GetProperties(BindingFlags.Public|BindingFlags.Instance);

       for (int i = 0; i < props.Length; i++) {
         if (props[i].Name == "AppStateId") {
           long val = Convert.ToInt64(props[i].GetValue(p, null).ToString());
           _appState =  (ABCState) val;
           break;
         }
       }
     }

    if (p is ABCWidget)  {
       _appState = (ABCState)((ABCWidget)p).AppStateId;
     }
  

Edoode:

Tim Gallagher:

private double DateDiff(string howtocompare, System.DateTime startDate, System.DateTime endDate)  {
   double diff=0;
   try
   {
     System.TimeSpan TS = new System.TimeSpan(startDate.Ticks-endDate.Ticks);

     #region converstion options
     switch (howtocompare.ToLower())
     { 

< snip >

I have seen this code in a production system before. A little Google finds (among others)

http://www.aspfree.com/c/a/ASP.NET/DateDiff-function-in-C-to-simulate-VBScripts-by-Robert-Chartier/

I think someone liked the VB style DateDiff() and other just copied it in...

 

  -Edoode

 

 

 

someone find that stupid SOB who created that function and smack them. They give developers a bad name and fill up internet searches with crap! 

It's not just quarters that's buggy.  Dividing days by 365 won't get you years, either.

I like how you can pass in anything at all for the "how to compare" string and get back days.  :)

Anonymous:

Tim Gallagher:

    System.TimeSpan TS = new System.TimeSpan(startDate.Ticks-endDate.Ticks);

Looks buggy to me as well. Doesn't this create a negative time span?

Yeup... Sure 'nuff will.
 

I try to avoid anything written by Robert Chartier like the plague.  Try reading some of his other articles on that same site.  Yikes.

Wheres the XML? This CodeSOD is lame.

captcha: awesomeness

Anonymous:

I try to avoid anything written by Robert Chartier like the plague.  Try reading some of his other articles on that same site.  Yikes.

 

Show me a project written by you and I shall find reason to have you hanged. A good programmer is writes good code, no programer will always take the correct path. Judge by the frequence of errors, not by one stupid mistake. 

 

edit: (this is meant as a general comment about programmers while referring to a 17 century philosopher, not toward Robert Chartier ) 

achille:

Anonymous:

I try to avoid anything written by Robert Chartier like the plague.  Try reading some of his other articles on that same site.  Yikes.

 

Show me a project written by you and I shall find reason to have you hanged. A good programmer is writes good code, no programer will always take the correct path. Judge by the frequence of errors, not by one stupid mistake. 

 

edit: (this is meant as a general comment about programmers while referring to a 17 century philosopher, not toward Robert Chartier ) 

Oh, I guess that means every crappy programming article I read on the intarweb is okay.  Just like that one author that didn't believe the stack existed unless you threw an exception.

if (p.GetType().ToString() ==

"Cxxxx.Cxxxx.Cxxxx.Wxxxx.AxxxxMxxxxBxxxx")

 {

   Type myType = p.GetType();

   PropertyInfo[] props =

myType.GetProperties(BindingFlags.Public|BindingFlags.Instance);

   for (int i = 0; i < props.Length; i++)

   {

     if (props[i].Name == "AppStateId")

     {

       long val = Convert.ToInt64(props[i].GetValue(p, null).ToString());

       _appState =  (enmCCLAppState) val;

       break;

     }

   }

 }

After studying the above code for a while to figure out what it did, the discoverer couldn't help but rewrite into a more efficient way to access a property:

if (p is AxxxxMxxxxBxxxx)  {

   _appState = (enmCCLAppState)((AxxxxMxxxxBxxxx)p).AppStateId;

 }

As bad as it may look, there are reason why this code is sometimes needed.  First of all, this is C# code, so for all you Java programmers, stop picking on '==' because it 
is valid in C#.  Second, the solution suggest would only works if AxxxxMxxxxBxxxx is listed as a reference when you build this code.  If the type AxxxxMxxxxBxxxx 
is dynamically loaded into the program during runtime, the suggest code won't even compile since the type does not exist in the build time.  Now one might wonder why 
don't we just put AxxxxMxxxxBxxxx as a reference in the project.  We could do that except Visual Studio does not allow circular dependency on project/assembly.  The most
elegant solutin for this is to abstract out AppStateId as an interface and have AxxxxMxxxxBxxxx implement that interface. But there are times where people just
don't have access to the code for AxxxxMxxxxBxxxx so such hack is required to get everything working. 

All it needs is the beanbag girl back.

versatilia:

TRWTF is that there's actually a company which actively finds and sorts out WTFs.

Doing years, quarters, minutes, seconds but not hours... that made me laugh out loud

 

 

trwtf is that you acronymed trwtf

 

CAPTCHA: WTF 

Re the reflection WTF: even if the class is loaded from a plugin at runtime it's still a WTF imho because it's hard to read. Either the class should implement an interface which is defined in the main assembly (in which case you could do

IWhatever thing = thing_to_test as IWhatever;
if(thing != null) _appstate = thing.AppState;

...) or the code to read a property via reflection ought to be refactored into a function and called:

_appstate = (AppState)ReadRuntimeProperty("Initech.Components.Whatever", "AppState");
 

Another RWTF is that he didn't bother adding 'h' as a case in the switch statement. It was a private function anyway, so he could just add it there. Seems like the right place to put it.

Tim Gallagher:

Over time all development teams inevitably gather their share of WTF code; at the very large project I am currently working on, they hand out "Van Gogh" awards for the best.

Augh! My ears! 

> I try to avoid anything written by Robert Chartier like the plague...

Personally, I try to avoid anything written by Albert Camus like, "The Plague."

I'm curious why they're called the Van Gogh Awards?

 

 

Anonymous:

Apart from issues already mentioned, what I really hate about this
code is that it offers the opportunity to provide random strings for
HowToCompare, and then doesn't tell you if you got it wrong; instead it
just returns some answer that may or may not have any relationship to
the value you wanted.

Why not use an enum? And why not stick an "assert (false)" in that default clause?

Agreed.  The fragility and almost-nonexistent error handling and is guaranteed to cause a nasty bug months or years down the road.

Of course, it's a WTF to use strings or enums or ints or anything else for this in the first place.  Stop using kewl abbreviations and make seven different functions already (DifferenceInDays, etc.).  No, the program will not be noticeably larger.  Every professional knows that simple code is better than elaborate code.

Bob Janova:

Re the reflection WTF: even if the class is loaded from a plugin at runtime it's still a WTF imho because it's hard to read. Either the class should implement an interface which is defined in the main assembly (in which case you could do

IWhatever thing = thing_to_test as IWhatever;
if(thing != null) _appstate = thing.AppState;

...) or the code to read a property via reflection ought to be refactored into a function and called:

_appstate = (AppState)ReadRuntimeProperty("Initech.Components.Whatever", "AppState");
 

 

In a perfect world you are absolutely correct.  However, it's not like we always get to modify/build assemblies that we reference to. Yeah it needs clean up that's for sure.. Especially the part where it loops through all the properties in a for loop till it finds the right one.  The point I am trying to make is, the suggest solution might not actually work because it's not equivalent to the original code. 

Anonymous:

I really enjoy this : 

if (p.GetType().ToString() == "Cxxxx.Cxxxx.Cxxxx.Wxxxx.AxxxxMxxxxBxxxx")

Anyone for a "class renaming" party ??
 

<a href="http://en.wikipedia.org/wiki/Liskov_substitution_principle">Liskov</a> must be turning over in her grave. Oh wait, she's not dead yet.

Tim Gallagher:

The replacement code:

hoursSinceLastRefresh = Convert.ToDecimal((DateTime.Now - cachedOn).TotalHours);

 

I think the real WTF is that you have languages that don't make everyday tasks easier.  For example, the above in CF is:

hoursSinceLastRefresh = DateDiff('h',Now(),cachedOn);

It's much cleaner and easier to read then the replacement code.  Why doesn't the date class of the language provide a datediff function?  That's teh real WTF..  

 

Anonymous:

Tim Gallagher:

The replacement code:

hoursSinceLastRefresh = Convert.ToDecimal((DateTime.Now - cachedOn).TotalHours);

 

I think the real WTF is that you have languages that don't make everyday tasks easier.  For example, the above in CF is:

hoursSinceLastRefresh = DateDiff('h',Now(),cachedOn);

It's much cleaner and easier to read then the replacement code.  Why doesn't the date class of the language provide a datediff function?  That's teh real WTF..  

 

 

How can you possibly think that passing a string argument to a function for this is cleaner than using properties and overloaded operators?

You're probably just scared of the decimal conversion, but that's how strongly typed languages work. Maybe someday you'll learn one and understand.

 

Anonymous:

Tim Gallagher:

hoursSinceLastRefresh = Convert.ToDecimal((DateTime.Now - cachedOn).TotalHours);

 

I think the real WTF is that you have languages that don't make everyday tasks easier.  For example, the above in CF is:

hoursSinceLastRefresh = DateDiff('h',Now(),cachedOn);

It's much cleaner and easier to read then the replacement code.  Why doesn't the date class of the language provide a datediff function?  That's teh real WTF..

Are you so allergic to the subtraction operator that you need a function to do it for you? (DateTime.Now - cachedOn).TotalHours is clear to the rest of us, and the Convert.ToDecimal call above is just cruft (it was just carried over from the original code which had an inappropriate choice of data type — and even so it should've been a typecast). It'd be easier with hoursSinceLastRefresh as a double, though without additional information, I suspect they really wanted a TimeSpan:
timeSinceLastRefresh = DateTime.Now - cachedOn;

Anonymous:

Anonymous:

Tim Gallagher:

The replacement code:

hoursSinceLastRefresh = Convert.ToDecimal((DateTime.Now - cachedOn).TotalHours);

 

I think the real WTF is that you have languages that don't make everyday tasks easier.  For example, the above in CF is:

hoursSinceLastRefresh = DateDiff('h',Now(),cachedOn);

It's much cleaner and easier to read then the replacement code.  Why doesn't the date class of the language provide a datediff function?  That's teh real WTF..  

 

 

How can you possibly think that passing a string argument to a function for this is cleaner than using properties and overloaded operators?

You're probably just scared of the decimal conversion, but that's how strongly typed languages work. Maybe someday you'll learn one and understand.

 

 

I know many languages, both strongly and weakly typed.   I have no problem with the decimal conversion (other then it's an additional, step that should be implicit in the first place). 

I do think that this code would be much cleaner:

 

 

hoursSinceLastRefresh = DateTime.DateDiff('h',DateTime.Now, cachedOn);

It more clearly explains what you're trying to do.  You need to KNOW that using an overloaded - operator on the dates will give you another date on which you need to find the totalHours method.  It would be much easier for people to find the DateDiff function in the DateTime class... 

Overloaded operators are a pain in general, which is why java got rid of them...  along with pointers... but then again, you probably like buffer overflows... otherwise where would 99% of exploits come from?

Maybe someday you will learn a language that doesn't make coding dangerous, and which doesn't force you to do explicit conversions.  If it takes a computer a few extra cpu cycles to auto convert the value to whatever format I need, but saves me 10 keystrokes, I can live with that.  

 

 Captcha: 1337
 

Wanna know the scariest part about this code? It's in a module used to approve multi-million dollar business loans.

"Every day's a new day" lol

Here are the WTFs I see: 

private double DateDiff(string howtocompare, System.DateTime startDate, System.DateTime endDate)  {

Using a string for enumerated type; enums exist for a reason.

System.TimeSpan TS = new System.TimeSpan(startDate.Ticks-endDate.Ticks);

DateTime has a static operator- method, so (startDate - endDate) should work just as well. And unless they're switching the sign later this will result in a negative span. (Yes I know this has been pointed out.)

     #region converstion options

     switch (howtocompare.ToLower())

     {

       case "m":

         diff = Convert.ToDouble(TS.TotalMinutes);

         break;

       case "s":

         diff = Convert.ToDouble(TS.TotalSeconds);

         break;

       case "t":

         diff = Convert.ToDouble(TS.Ticks);

         break;

       case "mm":

         diff = Convert.ToDouble(TS.TotalMilliseconds);

         break;

       case "yyyy":

         diff = Convert.ToDouble(TS.TotalDays/365);

         break;

       case "q":

         diff = Convert.ToDouble((TS.TotalDays/365)/4);

         break;

       default:

         //d

         diff = Convert.ToDouble(TS.TotalDays);

         break;

     }

     #endregion

This is full of 'em:

• Using Convert instead of a cast (which will be faster unless the compiler replaces the static method call with a cast itself).


• Converting to double after division will result in an integer anyway. This method may as well declare to return int/long, it's not going to return anything but integers.

catch

   {

     diff = -1;

   }

Let the exception propagate, dammit! That's what exceptions are for. I'd hate to be the consumer of this method.

Aside from the real WTfery going on here.... what's with the fascination of passing an ascii string to describe the operation? Whatever happened to enums?

 

 

The point I am trying to make is, the suggest solution might not actually work because it's not equivalent to the original code.

You can always take the process of reading the function and abstract it into a function. I agree that you can't always make the class implement an interface of your choice, though it is best if you can do it.

Tim Gallagher:

if (p.GetType().ToString() ==

"Cxxxx.Cxxxx.Cxxxx.Wxxxx.AxxxxMxxxxBxxxx")

 {

}

if (p is AxxxxMxxxxBxxxx)  {

   _appState = (enmCCLAppState)((AxxxxMxxxxBxxxx)p).AppStateId;

 }

Uhm, I'm not exactly a C# guru, but are those two statements equivalent? Wont the second case generate some false positives? What happens if the user has Cxxxx.Cxxxx.Cxxxx.Wxxxx.AxxxxMxxxxBxxxx and Cyyyy.Cyyyy.Cyyyy.Wyyyy.AxxxxMxxxxBxxxx?

 

Anonymous:

It more clearly explains what you're trying to do. You need to KNOW that using an overloaded - operator on the dates will give you another date on which you need to find the totalHours method. It would be much easier for people to find the DateDiff function in the DateTime class...

lol

Anonymous:

Overloaded operators are a pain in general, which is why java got rid of them...  along with pointers... but then again, you probably like buffer overflows... otherwise where would 99% of exploits come from?

rofl