The Weird and The Wonderful

The Weird and The Wonderful forum is a place to post Coding Horrors, Worst Practices, and the occasional flash of brilliance.

We all come across code that simply boggles the mind. Lazy kludges, embarrassing mistakes, horrid workarounds and developers just not quite getting it. And then somedays we come across - or write - the truly sublime.

Post your Best, your worst, and your most interesting. But please - no programming questions . This forum is purely for amusement and discussions on code snippets. All actual programming questions will be removed.

Re: Converting a string to an int

PIEBALDconsult14-Dec-11 1:51

PIEBALDconsult

14-Dec-11 1:51

10!

BobJanova wrote:
one calls the other

Yes, Convert calls the specific class' method, Int32.Parse in this case -- that's why I tell people not to use Convert.

modified 15-Dec-11 11:18am.

Re: Converting a string to an int

Shameel14-Dec-11 3:41

Shameel

14-Dec-11 3:41

BobJanova wrote:
you have debugging set to look at caught exceptions?

yes, I have to do that to handle some subtle exceptions that are caught and swallowed (empty catch blocks - now that deserves a separate post by itself).

BobJanova wrote:
try adjusting your settings

I hate to keep doing it every now and then to accomodate this kind of code. Unfortunately, for now, I have to live with it since I cannot change the behavior of existing "working code".

Re: Converting a string to an int

BobJanova14-Dec-11 3:50

BobJanova

14-Dec-11 3:50

But my point is that 'this kind of code' is basically any code that uses exceptions.

Swallowed exceptions generally deserve a post here, yes. This code handles the exception to put the variable into a valid state so it's not in the same category.

Perhaps better than changing your settings would be to make the config file valid.

Documentation failure

DerekT-P13-Dec-11 3:17

DerekT-P

13-Dec-11 3:17

An app I support interfaces to a large number of 3rd party webservices. Often the documentation is poor or incomplete. Last week I integrated to one that seemed pretty well documented. At one point it says

xxxxxx can be called in one of four modes:
● Redirect (default)
● XML
● JSON
● iFrame
The mode can be specified by setting the 'mode' query string parameter

and subsequently refers to "XML mode" several times. I coded up the interface, ran some tests; and got non-specific errors every time. Eventually the 3rd party company got back to me with the following explanation:

mode=XML should be mode=xml i.e. the "xml" is case sensitive.

It works now but it's wasted two hours of my time and delayed the integration by the best part of a week.

TEST THE DOCUMENTATION TOO, YOU WALLYS!

Re: Documentation failure

GibbleCH13-Dec-11 4:29

GibbleCH

13-Dec-11 4:29

Agreed...but how does 2 wasted hours delay integration nearly a week?

Re: Documentation failure

DerekT-P13-Dec-11 4:52

DerekT-P

13-Dec-11 4:52

If does when they don't reply to your query for 5 days!

Re: Documentation failure

Nagy Vilmos13-Dec-11 5:27

Nagy Vilmos

13-Dec-11 5:27

As is our wont to point out, the API is wrong. Big Grin | :-D

It should be an enumeration and then case doesn't come into it OR it should be throwing a fracked exception saying "Unknown mode, must be 'redirect', 'xml', 'json' or 'iframe'."

Panic, Chaos, Destruction. My work here is done.
Drink. Get drunk. Fall over - P O'H
OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre
I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer
Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett

Re: Documentation failure

BobJanova13-Dec-11 23:42

BobJanova

13-Dec-11 23:42

Yeah, the documentation is wrong too but the API should be giving you a clear exception if you give it invalid input.

Re: Documentation failure

Nagy Vilmos13-Dec-11 23:54

Nagy Vilmos

13-Dec-11 23:54

Open API's should always work via 'design by contract'. As long as the pre-conditions are met, a valid post-condition will be reached, otherwise an exception is raised and the state is unchanged.

Re: Documentation failure

DerekT-P14-Dec-11 3:48

DerekT-P

14-Dec-11 3:48

Absolutely agree with you. However, regardless of the API itself, the documentation was remiss in not specifying what the actual parameters required are. "Test-driven development" does not mean sending data to the API to "see what happens"! Documentation is PART of the deliverables for virtually any project. Whilst we may not like it, documenting our stuff is vital and not only that, but the documentation needs testing too. By that I mean if you follow the documentation, without any pre-existing knowledge, is the outcome correct? Whether the API returns just "Invalid" or "Invalid; should be one of 'xml','redirect' ..." etc doesn't change the fact the documentation is just plain wrong, by omission in this case. The depressing thing is this isn't new or unusual; I blogged about this over 2 years ago http://www.smallofficesolutions.co.uk/wordpress/index.php/2009/05/11/acuracy-is-improtant/[^] and in that case the API itself was returning a message referring to "missing field XmlData" whereas the documentation referred to "xmlData" (and the documentation was right in that instance, the API was wrong!)

Re: Documentation failure

Sander Rossel20-Dec-11 7:59

Sander Rossel

20-Dec-11 7:59

From another perspective: we got some requirements from a customer on what we should build for them.
They want a Form with lots of information on many tabs.
The documented description of a tab, for example the 'customer' tab: "Shows info on the customer"... Really? I'm glad we got requirements for that!
This was written by people who specialize in writing those documents.
I guess writing good documents just isn't easy.

It's an OO world.

public class Naerling : Lazy<Person>{
    public void DoWork(){ throw new NotImplementedException(); }
}

Re: Documentation failure

BillW3321-Dec-11 3:05

BillW33

21-Dec-11 3:05

Over the years I have seen lots of bad documentation; from missing specs to APIs that are just plain wrong. I am no longer surprised when the docs do not match what the app actually does. Apparently documentation is an afterthought at many companies. Sigh | :sigh:

Just because the code works, it doesn't mean that it is good code.

Do you not understand booleans?

alanevans6-Dec-11 1:11

alanevans

6-Dec-11 1:11

This stuff drives me up the wall!!!

bool is_queue_empty(void)
{
    if (queue_length==0)
    {
        return true;
    }
    else
    {
        return false;
    }
}

Or this:

bool counter_zero = counter==0 ? true : false;

Or this:

if (isUDPSetup()==true)
{
	if ((forceSend==false))
	{
		...
	}
}

(Variable names have been changed to protect the guilty)

Or this *New one*:

void setNeedsUpdate(bool update)
{
  if ((update==true))
    NeedsUpdate=true;
  else
    NeedsUpdate=false;
}

modified 6-Dec-11 9:06am.

Re: Do you not understand booleans?

CDP18026-Dec-11 1:49

CDP1802

6-Dec-11 1:49

You are a lucky man if that's all that makes your life miserable. Some of this is awkward, especially the second example, but where exactly is the horrible part? What damage is done? Does it not work or are there possible side effects? Does it lead to worse code being generated by the compiler?

I think the first two just show a clumsy coding style, but no real harm comes from it. Except, of course, curling up your toenails when you have to read it.

Usually I do the last one myself. It is at least the same way you compare any other variable (if (x == 0), so why not if (flag == true)?) and I prefer to make it absolutely clear what I want to accomplish. The compiled code remains absolutely the same, no matter how you write it. That's why I see the last one more as a matter of preferences, but not as anything important.

And from the clouds a mighty voice spoke:
"Smile and be happy, for it could come worse!"

And I smiled and was happy
And it came worse.

Re: Do you not understand booleans?

harold aptroot6-Dec-11 2:36

harold aptroot

6-Dec-11 2:36

Screw the compiler. It takes longer to read and understand, therefore it sucks.

Re: Do you not understand booleans?

Fabio Franco12-Dec-11 8:05

Fabio Franco

12-Dec-11 8:05

And it obviously makes the reader scared of what else might come from that code who's developer can't understand even a booleans...

"To alcohol! The cause of, and solution to, all of life's problems" - Homer Simpson

Re: Do you not understand booleans?

mostlyharmless196414-Dec-11 3:15

mostlyharmless1964

14-Dec-11 3:15

i might be showing my age here but there used to be a readability test known as "the telephone test" (from Kerningham and Plauger) - Read your code to someone over the phone. If they can't understand it, try writing the code again. admittedly this has its problems but one of the upshots was that you should just name your booleans for the thing they test and then it reads well. also, comparing to "== true" or "== false" obviously breaks this readability test Smile | :)

Re: Do you not understand booleans?

Fabio Franco14-Dec-11 4:06

Fabio Franco

14-Dec-11 4:06

mostlyharmless1964 wrote:
i might be showing my age here

Wow, never heard of it. Was it the time when the compilers were birds inside stone cases? Laugh | :laugh:

"To alcohol! The cause of, and solution to, all of life's problems" - Homer Simpson

Re: Do you not understand booleans?

alanevans6-Dec-11 2:58

alanevans

6-Dec-11 2:58

The harm comes entirely from maintainance, more code == more bugs or at least harder to spot them.

I apreciate your style preference on the last one, and the last thing I want is to start a style argument, but I am curious, would you also do this if you had two flags?:

if((flag1==true) && (flag2==true))
{
  ...
}

Because this guy does, and they get longer and longer because of all the ==true and ==falses, makes simple boolean expressions look very complicated.

Re: Do you not understand booleans?

CDP18026-Dec-11 3:26

CDP1802

6-Dec-11 3:26

Yes, I suppose this simply comes from the old C++ days where there was no boolean type. To avoid side effects or compiler specific behavior, I always explicitly specified what I was testing.

But I would write it like this:

if((flag1 == true) && 
   (flag2 == true) && 
   (flag3 == true))
{
  ...
}

Usually it does not look so uniform. If I really had so many different flags, I would think about using a flag word and testing all flags in one go.

And from the clouds a mighty voice spoke:
"Smile and be happy, for it could come worse!"

And I smiled and was happy
And it came worse.

Re: Do you not understand booleans?

alanevans6-Dec-11 3:54

alanevans

6-Dec-11 3:54

Well this kind of code can be even more dangerous in C where you have #defined your true and false constants (dont' know if it's same in C++).

Now you can get the situation where potentially neither section 1 or 2 runs:

if(a==true)
{
  //1
}

if(a==false)
{
  //2
}

At least if you don't use the constants, it behaves as a boolean, here either section 1 or 2 is guaranteed to run.

if(a)
{
  //1
}

if(!a)
{
  //2
}

Re: Do you not understand booleans?

CDP18026-Dec-11 4:16

CDP1802

6-Dec-11 4:16

Fine. Now what if a is a (signed) integer and has a negative value? Or what if a is a pointer which is currently NULL? Without having defined any value for TRUE or FALSE and without knowing how NULL was defined somewhere deep in the libraries, how do you now know which code will be executed and which not? Even if NULL is usually defined as 0x00, you cannot expect this to be true for every compiler. And what can happen if you use another compiler?

int*   a = NULL;
int    b = -42;


if(a)
{
    // We should not need to know how NULL is defined and therefore can never know wether
    // or not this code block will ever be entered
}

if(a == NULL)
{
    // Now we explicitly compared with NULL and it is clear when this code will be executed
}

if(b)
{
    // Negative values are undefined and it is up to the compiler wether a negative value is
    // seen as 'true' or as 'false'
}

if(b < 0)
{
    // Explicitly testing the variable again removes all uncertainties
}

And from the clouds a mighty voice spoke:
"Smile and be happy, for it could come worse!"

And I smiled and was happy
And it came worse.

Re: Do you not understand booleans?

alanevans6-Dec-11 4:55

alanevans

6-Dec-11 4:55

I am with you that if it's an integer or pointer do the code like you have described, it's fine code, I do prefer to do it this way. But you still arn't using defines for true and false, which is my original beef.

However any compiler that doesn't define zero as false and non-zero as true for boolean expressions is not a standard C compiler.

Re: Do you not understand booleans?

CDP18026-Dec-11 5:29

CDP1802

6-Dec-11 5:29

The simple answer: I would, for example, use the Win32 type BOOL. I can safely compare any variable of the type BOOL to the definitions of TRUE or FALSE and that is what I would do. Trying to compare any variable of another type with TRUE or FALSE would be asking for trouble. Also, I would not use other types to express boolean values, even if C/C++ allows this, so that there is never any need to compare them to TRUE or FALSE

BOOL b = TRUE;
int i = 1;

if(i == TRUE)
{
    // TRUE may be defined in numerous ways, so this is likely not to work
    // I would simply not use an int (or other types) to express boolean values.
}

if(b)
{
    // It would be logical to assume that this works, but who knows how TRUE has
    // really been defined. In any case, we should not have to know and that
    // is why I avoid it.
}

if(b == 1)
{
    // TRUE (the value of b) may be defined in numerous ways, so this is likely not to work
}

if(b == i)
{
    // Very bad. Even if the definition of BOOL is based on int, it's unsafe to assume that this is so. And again
    // we should not have to care about how TRUE or FALSE are defined.
}

if(b == TRUE)
{
    // No problem here. Explicitly comparing a BOOL variable with TRUE or FALSE seems to be the only safe way.
}

Coming back to managed languages where a boolean type exists: Here no other type can be used to express boolean
values, so explicitely comparing to true or false is not needed. I guess it's simply a habit I have carried over
without thinking much about it.

And from the clouds a mighty voice spoke:
"Smile and be happy, for it could come worse!"

And I smiled and was happy
And it came worse.

Re: Do you not understand booleans?

PIEBALDconsult9-Dec-11 1:55

PIEBALDconsult

9-Dec-11 1:55

alanevans wrote:
define zero as false

What size of zero? Big Grin | :-D

Even defining it as zero may be problematic down the line. One place I worked (which had to compile its code for 16-bit DOS, 32-bit Windows, and 64-bit OpenVMS used:

# define true (0==0)

And were therefore protected against issues of size, or whether zero was true or false, or even if some future compiler has an actual boolean type. That, in my opinion, is the only way to go.

Use Ctrl+Left/Right to switch messages, Ctrl+Up/Down to switch threads, Ctrl+Shift+Left/Right to switch pages.