|
He was trying to copy a subset of xml data from one document to another (how this can be useful beats me unless you unload the document containing the superset of data, but anyway that's what he tried to do) but the data would not have the same nodes in all cases. Apparently upon discovering this he figured he'd just wrap the code copying each node into a try-catch so it continues if a node doesn't exist.
This is a lazy approach and a coding horror in my book. First, the cost of catching exceptions instead of checking if nodes exist before trying to clone them is *huge*. Second, in most cases the try block can fail for other reasons than the one which "is ok" and means you should just go on. This project is plagued by that particular problem, people catch an exception because they want the program to "keep working" but of course all it does is make it fail in strange ways while leaving no useful debugging information. It's worse when they do this server-side since the client-side exceptions aren't so rich on information anyway but the principle is the same.
My real gripe however is the code redundancy. If you intend to perform the same operation 20 times over you should use the same implementation 20 times, so it's possible to change the darn thing without getting bored to death.
I also think that if the operation is of a fairly generic nature, say, copying a subset of xml nodes from one document to another, you should separate the logic from any particular bits of the systems such as, say, a specific web page in a complex application. The guy is now Chef de projet développements (not for my team thankfully) and continues to wreak havoc at undiminished speed.
An example of what he might have written instead:
function copyChildNodes(srcNode, destNode, nodeList, ignoreMissingNodes)
{
for (var i=0; i < nodeList.length; i++)
{
var node = srcNode.selectSingleNode(nodeList[i]);
if (!node)
{
if (ignoreMissingNodes)
continue;
else
throw {message: "No child node with name '" + nodeList[i] + "' exists in the source node."};
}
destNode.appendChild(node.cloneNode(true));
}
}
Put that in one of our several script files intended for code a little bit more general than the page-specfic stuff and put this in the page (instead of the twenty lines doing the same thing):
var nodes = ["block_name", "block_type", "title", "list_id", "you", "get", "the", "point"];
copyChildNodes(oXMLBlock, oXMLRecord, nodes, true);
Surely you can see how this second approach allows a long-term maintenance of the code that is completely incomparable to the copy-paste approach. I like to call it the "cut-and-paste" approach actually, because in practice it often starts with the realization that we're writing code in the wrong place, that a lot of what we're coding is not specific to the task and should be refactored to allow us to reuse it. This refactoring usually starts with cutting out part of the code and moving it somewhere else, then perhaps cleaning it up a bit or parameterizing some aspect of the behavior, such as "ignoreMissingNodes" in the above example. Reuse is extremely important, not just to save time (copy-paste achieves that, at first development, nearly as well), but to fix bugs and improve the QUALITY of the code base over time.
|
|
|
|
|
dojohansen wrote: My real gripe however is the code redundancy. If you intend to perform the same operation 20 times over you should use the same implementation 20 times, so it's possible to change the darn thing without getting bored to death.
What are you talking about, that just means you get to have fun writing an overly complex regex to update the code with. 
Today's lesson is brought to you by the word "niggardly". Remember kids, don't attribute to racism what can be explained by Scandinavian language roots.
-- Robert Royall
|
|
|
|
|
Seen something very similar from a guy who sells the source code of his application for more than 2000 dollars. He said that using try catch blocks like it's going out of fashion is "covering every base".
...the horror. 
|
|
|
|
|
Sadly, had a boss one time that changed my code from using an array and a loop_index to code that looks like the example. Perhaps we were being paid per line of source code generated?
|
|
|
|
|
Horror 1 :
if (strRet[0] == '0' && strRet[1] == '1' &&
strRet[2] == 'F' && strRet[3] == 'F' &&
strRet[4] == '2' && strRet[5] == '4')
A piece of code I just looked at had the preceding logic sprinkled through it around twenty times to examine the response from a piece of equipment with comparisons of varying lengths. Apparently this individual has never heard of strncmp or its TCHAR variant _tcsncmp.
Horror 2 :
They wrote their own function essentially equivalent to strtoul. I won't subject you all to it - you'll have to trust me when I say that it is a horror.
Horror 3 :
The function described in Horror 2 is called ConvertHEXASC(). Here's how it is used to convert the data from the equipment :
int iTmp = 0;
iTmp += ConvertHEXASC( strRet[7] ) * 1;
iTmp += ConvertHEXASC( strRet[6] ) * 16;
iTmp += ConvertHEXASC( strRet[5] ) * 256;
iTmp += ConvertHEXASC( strRet[4] ) * 4096;
Now imagine a similar sequence of code used a few dozen times with strings of varying lengths and positions. 
Horror 4 :
There were sixteen buttons used to select a property for the equipment and, naturally, there were sixteen different functions made to handle the clicks of the buttons. ON_COMMAND_RANGE anyone ? 
The word horror barely scratches the surface for these and the rest of the things I saw there.
It was staggering. Simply staggering.
|
|
|
|
|
What's the background of such champion-of-code-development?
Just curious. 
If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong.
-- Iain Clarke
[My articles]
|
|
|
|
|
There were sixteen buttons used to select a property for the equipment and, naturally, there were sixteen different functions made to handle the clicks of the buttons. ON_COMMAND_RANGE anyone ?
And when you add another button, and the seventeenth ID is already used for something else, you have a whole heap of work to do.
|
|
|
|
|
Graham Bradshaw wrote: And when you add another button, and the seventeenth ID is already used for something else, you have a whole heap of work to do.
There's no problem pointing multiple ON_COMMAND_RANGE macros to the same function.
"Multithreading is just one damn thing after, before, or simultaneous with another." - Andrei Alexandrescu
|
|
|
|
|
I do not consider renumbering the values in Resource.h to be a whole heap of work. Besides, I always leave gaps around arrays of control identifiers just in case I have to add more so this is very rarely an issue for me and when it arises it is easy to deal with.
|
|
|
|
|
I've found this piece of code recently in my new project
if(a==1 || a==2 || a==3 || a==4)<br />
{<br />
if(a==1)<br />
DoSomething(1);<br />
else if(a==2)<br />
DoSomething(2);<br />
else if(a==3)<br />
DoSomething(3);<br />
else if(a==4)<br />
DoSomething(4);<br />
}
And this part is written in many places of the application
|
|
|
|
|
now THAT'S a true horror.
|
|
|
|
|
Yes, it is. Hopefully the coder is no longer with that company.
"The clue train passed his station without stopping." - John Simmons / outlaw programmer
"Real programmers just throw a bunch of 1s and 0s at the computer to see what sticks" - Pete O'Hanlon
"Not only do you continue to babble nonsense, you can't even correctly remember the nonsense you babbled just minutes ago." - Rob Graham
|
|
|
|
|
I hope your using the term "Coder" loosely there... Probably better said by saying **** Monkey, with approporiate word inserted in place of Code..
|
|
|
|
|
Does you company pay for lines of code?
If the Lord God Almighty had consulted me before embarking upon the Creation, I would have recommended something simpler.
-- Alfonso the Wise, 13th Century King of Castile.
This is going on my arrogant assumptions. You may have a superb reason why I'm completely wrong.
-- Iain Clarke
[My articles]
|
|
|
|
|
I would definetly make that suggestion in the next apprisal cycle
|
|
|
|
|
You need to suggest doing stricter code reviews, if your company doesn't already do so.
"The clue train passed his station without stopping." - John Simmons / outlaw programmer
"Real programmers just throw a bunch of 1s and 0s at the computer to see what sticks" - Pete O'Hanlon
"Not only do you continue to babble nonsense, you can't even correctly remember the nonsense you babbled just minutes ago." - Rob Graham
|
|
|
|
|
My humble advice to you is: CHANGE THE COMPANY!!!
The narrow specialist in the broad sense of the word is a complete idiot in the narrow sense of the word.
Advertise here – minimum three posts per day are guaranteed.
|
|
|
|
|
Narotham Babu Kalluri wrote: this part is written in many places of the application
That's too bad. What was the programmer thinking?
"The clue train passed his station without stopping." - John Simmons / outlaw programmer
"Real programmers just throw a bunch of 1s and 0s at the computer to see what sticks" - Pete O'Hanlon
"Not only do you continue to babble nonsense, you can't even correctly remember the nonsense you babbled just minutes ago." - Rob Graham
|
|
|
|
|
I think he'd switched off 
Regards,
--Perspx
"I've got my kids brainwashed: You don't use Google, and you don't use an iPod." - Steve Ballmer
"Some people have told me they don't think a fat penguin really embodies the grace of Linux, which just tells me they have never seen an angry penguin charging at them in excess of 100mph." - Linus Torvalds
|
|
|
|
|
Perspx wrote: switched
Nice play on word there 
"The clue train passed his station without stopping." - John Simmons / outlaw programmer
"Real programmers just throw a bunch of 1s and 0s at the computer to see what sticks" - Pete O'Hanlon
"Not only do you continue to babble nonsense, you can't even correctly remember the nonsense you babbled just minutes ago." - Rob Graham
|
|
|
|
|
Thanks 
Regards,
--Perspx
"I've got my kids brainwashed: You don't use Google, and you don't use an iPod." - Steve Ballmer
"Some people have told me they don't think a fat penguin really embodies the grace of Linux, which just tells me they have never seen an angry penguin charging at them in excess of 100mph." - Linus Torvalds
|
|
|
|
|
It seems that someone simply has knowledge leaks. Knowing that it is possible to pass a value of a variable to a function directly not like this, is bloody essential thing and it would be nice if someone told him or her that. Just snoop around to find out who wrote this and make a good deed today. Hmm.. it's a funny horror anyway.
Greetings - Gajatko
Portable.NET is part of DotGNU, a project to build a complete Free Software replacement for .NET - a system that truly belongs to the developers.
|
|
|
|
|
Narotham Babu Kalluri wrote: I've found this piece of code recently in my new project
if(a==1 || a==2 || a==3 || a==4)
{
if(a==1)
DoSomething(1);
else if(a==2)
DoSomething(2);
else if(a==3)
DoSomething(3);
else if(a==4)
DoSomething(4);
}
Since this programmer obviously can't make a switch in code, your company should make a switch of employees.
|
|
|
|
|
Someone voted you down for that. Gave you a 5 to balance it out 
"The clue train passed his station without stopping." - John Simmons / outlaw programmer
"Real programmers just throw a bunch of 1s and 0s at the computer to see what sticks" - Pete O'Hanlon
"Not only do you continue to babble nonsense, you can't even correctly remember the nonsense you babbled just minutes ago." - Rob Graham
|
|
|
|
|
Paul Conrad wrote: Someone voted you down for that. Gave you a 5 to balance it out
Thanks
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
