Wednesday, January 17, 2007

How not to write an IVR

So for my first post here, let me take a minute to tell you a little about my most recent project at work. We're putting together an IVR for one part of a very large suite of applications. This suite of applications was supposed to be released on January 1st, but of course now it's January 17th and we're only just now getting to true integration testing.

Unfortunately we outsourced the IVR software to supposed "experts," which is what landed us in trouble. Their technical person who has been writing the software that controls the IVR script is a complete and utter tool. He doesn't know the basics of C++ let alone software development in general.

For instance, last night I sat on the phone with him for an hour-and-a-half whilst I debugged his code for him. I should point out that I am by trade a web designer and a managed code developer that normally uses Microsoft .NET. I have not touched C++ in about ten years, so I was using Google a lot and still filling in holes in his code. Meanwhile, the only insight he really offered was, "Well I can't imagine why it would be doing that!" Well, duh, that's why you're crappy code doesn't work, Einstein.

Basically the IVR script iterates through a bunch of prompts, gathering data from the caller, and posts each response to a web service. Should be pretty straightforward, right? He was convinced that it was my web service and not his code that could be the problem. Eventually I tracked it down to one prompt in the IVR that was destabilizing the entire application and causing it to crash (eventually crash, that is, which is why it was so damn hard to find). Code similar to the following was the culprit:

char filename[200];
memcpy(filename, basepath, 200);


What's wrong with this? We're copying one string into another, but the destination is not initialized or allocated to anything. He's literally writing bytes into no-man's land. Pointers and memory allocation are a hard concept to grasp for most beginning programmers, granted, but someone who works for a company writing C++ code full-time and charging ungodly amounts of money for that time should not be making such elementary errors.

Once I fixed this error for him, the program stopped crashing. Luckily I had hung up on him by that point, so he avoided my wrath.

While I'm at it, here's one more gem of his code (excuse me if it's not perfectly compilable, I'm doing this from memory):

j = 0;
for (i = 0; i < strlen(promptfiles); i++)
{
if (promptfiles[i] == '|')
{
memcpy(prompts[prompt_index], promptfiles + j, i - j);
j = i + 1;
prompt_index++;
}
}


This little nugget was meant to parse a string of IVR prompt sound files I was giving to him from the web service in a pipe-delimited format like:


Prompt1Name|ivr_prompts\prompt1.wav|Prompt2Name|ivr_prompts\prompt2.wav|...


According to him there was a problem with one of our prompts called "Timeout" that wouldn't play correctly. Can you spot the problem in his code? I'll give you a hint; "Timeout" appeared at the end of the pipe-delimited string.

The answer is pure lack of brainpower on his part. He only copies the part of the string he wants after he hits a pipe delimiter. The string wasn't terminated by a pipe, so he was just dropping off the last part of the string and not saving it.

I guess the real mystery is why he didn't just use strtok.

Anyways, that's my rant for this evening. I promise I'll start putting some happier and more informative posts here later; for now I just needed to get this off my chest.

No comments: