|
I'm sorry, but I fail to see the funny side. How is the person who takes over from the pair of you going to think when he finds this code? I expect we'll see it posted here again as an example of a code fail. Yes, the original code was wrong, but you have just made the situation 10 times worse, and potentially stored up a huge problem for your employer in the process.
The fact that you did this deliberately is shocking.
|
|
|
|
|
Pete O'Hanlon wrote: How is the person who takes over from the pair of you going to think when he finds this code?
He won't. It's fixed and my colleague announced. Actually he did find it somewhat funny.
Pete O'Hanlon wrote: Yes, the original code was wrong, but you have just made the situation 10 times worse, and potentially stored up a huge problem for your employer in the process
I fail to see how I created a huge problem. I did not change the data or interact with it in any way (except fixing what came wrong), I logged something badly instead of not logging it at all, and whoever used the VS debugger would notice it in a flash.
Pete O'Hanlon wrote: The fact that you did this deliberately is shocking.
Although I do acknowledge it was uninspired and totally unprofessional (I try to take myself seriously, as well as others, and here I was wrong), shocking no, I don't think so.
Again, I was wrong for this, and thanks to the CP community for pointing it out.
EDIT: Should I delete the thread, or leave it as is?
Full-fledged Java/.NET lover, full-fledged PHP hater.
Full-fledged Google/Microsoft lover, full-fledged Apple hater.
Full-fledged Skype lover, full-fledged YM hater.
|
|
|
|
|
Leave it. Hopefully others can learn from it. Sometimes a quiet word is the right solution to a problem.
|
|
|
|
|
I have to agree with Pete. The feeling I get reading this thread is that your coworker wrote some bad code out of stupidity/stubbornness/inexperience and you countered that with malice.
Shocking doesn't begin to describe it to me. If that code got checked into the repository, you're fired.
Sorry to be so harsh, but this is a completely inappropriate way to handle the situation.
I have to say, I am in total agreement with you on your notion that logging every caught exception is the proper way to go. Even if your code can continue on properly, it's good to log that. To give a trivial example:
int num = 0;
try {
num = Convert.ToInt32(string);
} catch (FormatException e)
Log.Info("Couldn't convert " + string + " to int, using default");
num = <some setting from config>;
}
Of course, this is trivial, but you get the idea.
|
|
|
|
|
The problem with the simple example is that the catch could blow up throwing you out of your process completely. (The passed "string" is null. The Info function should throw because it was passed null...) What if the string is a few thousand characters? I wouldn't use data passed to the process in the message either.
Probably something like
Log.Info("Using default value for \"ConvertToInt\" process because of msg:" + e.Message);That isolates what caused the problem from the log process. And may actually tell the user what went wrong and why.
Your user guide would, of course  , tell the user what ConvertToInt does, how they might encounter it and what the default value would be.
I can certainly understand the orig. poster's frustration, he has a friend who's being thick-headed in one area. He used a humorous way to illistrate that no message could be nearly as damaging as intentionally sending a misleading message.
|
|
|
|
|
1. A programming language that would even let you do this is broken. I feel so unclean just having read this.
2. Type that into the codebase, and it's only a matter of time before you are the victim; either of forgetting to take it out, or having it used against you.
3. Pranks. Oy! I've worked in organizations where pranks got to be the accepted norm. They are sick and unprofessional organizations. Generally what happens is somebody goes too far. Maybe there is an injury, or a customer gets pranked by accident. Then there is a short, grim meeting where your boss or your division manager or somebody from Personnel lays it on the line saying, "Next prank gets you fired."
A better approach is to make the dumb behavior (not logging) an issue with your colleague, and if he doesn't make an attempt to improve, with your team or your manager. Because your colleague can certainly make the pranking an issue with your manager. Then he will have the moral high ground and you will lose traction on getting the undesirable behavior fixed. If you act unprofessionally because your teammate did, that fixes nothing.
|
|
|
|
|
Quote: "A programming language that would even let you do this is broken." -SeattleC++
.
Have you ever actually used C++, at all? I'm guessing no, otherwise you'd realize that it's quite capable of doing truly horrible things...
|
|
|
|
|
Joe_Dert wrote: Have you ever actually used C++, at all? I'm guessing no, otherwise you'd realize that it's quite capable of doing truly horrible things...
This is hardly to the point. And yeah, I've used C++. Since you were in middle school, I'm guessing.
It may well be true that it's a poor workman who blames his tools. But it's also true that it's a deranged colleague who thinks up ways to deliberately damage shared tools and then checks their perverse thoughts in. And then brags about it.
|
|
|
|
|
You're dodging the issue, he did something devious, yes, but, that wasn't my point.
Pay attention, you said that any language that allowed him to do what he did was broken, to which I responded(more or less), that C++ is far worse in that regard.
(ie: By your reasoning, C++ is broken, because it has many mechanisms which can be abused to do devious things.)
Furthermore, I'd suggest that you take some of your "assumed" wisdom of years, and at least try act the part of the wise older man if you want to play the: "I've been coding since you were in diapers" card.
Let's be frank, I had quoted you, so, assuming you speak English, you know what I meant, so, either you're a troll, illiterate, or perhaps just senile?
Best case scenario, I'm willing to conclude there was some sort of misunderstanding.
modified 22-Oct-12 2:44am.
|
|
|
|
|
I'm going to have to join the crowd who say that: yes, this is funny, and devious, but if that made it to the source control for an actual release then you're a bad developer too.
Also I'm pretty sure you'll get a compiler warning for hiding inherited method GetType which should be a bit of a giveaway.
And honestly if it was only 15 minutes that you wasted tracking it down then it's not worth getting annoyed by.
|
|
|
|
|
BobJanova wrote: but if that made it to the source control for an actual release then you're a bad developer too
No, he and I have a branch especially made for stuff that's either highly unstable, proofs of concept or simply little experiments.
BobJanova wrote: Also I'm pretty sure you'll get a compiler warning for hiding inherited method GetType which should be a bit of a giveaway.
My little shenanigan wasn't meant to stand anything but superficial scrutiny anyway, which I mentioned in the original post. Any debug through the code would've revealed it immediately.
BobJanova wrote: And honestly if it was only 15 minutes that you wasted tracking it down then it's not worth getting annoyed by.
Yes, I did get pretty carried away. I don't normally do this, nor do I intend to do it anytime soon, but this time I slipped.
Full-fledged Java/.NET lover, full-fledged PHP hater.
Full-fledged Google/Microsoft lover, full-fledged Apple hater.
Full-fledged Skype lover, full-fledged YM hater.
|
|
|
|
|
Although i love to play pranks, i consider that if this have the remote possibility to reach production, then you're fried (and fired) and as a matter of fact, if your buddy doesn't log errors, i think very unlikely that he sees the log.
|
|
|
|
|
Yes, both your points are valid, and I already replied to them in other posts above.
Full-fledged Java/.NET lover, full-fledged PHP hater.
Full-fledged Google/Microsoft lover, full-fledged Apple hater.
Full-fledged Skype lover, full-fledged YM hater.
|
|
|
|
|
maybe iam missing something, but if you log e.ToString(), then you have both class name and method name plus line of the code the exception happend.In my helper i just log e.ToString.Any reason?
Behzad
|
|
|
|
|
No, we are logging class name, method name, and exception message. We also sometimes log stack traces for errors that tend to get funky, but usually just logging the exception message is enough for us.
Codebases are not that great (we do lots of small applications instead of a large one that does everything), and it's pretty easy to figure what went wrong and why, without explicitly needing a StackTrace for it. That's exactly why it's optional, for cases where it's not immediately clear what goes wrong. As you said, we could log the whole error, but for most cases it would be overkill.
Full-fledged Java/.NET lover, full-fledged PHP hater.
Full-fledged Google/Microsoft lover, full-fledged Apple hater.
Full-fledged Skype lover, full-fledged YM hater.
|
|
|
|
|
Andrei Straut wrote: That should teach them to log stuff properly, or use the debugger
Unless of course it is you that encounters it 6 months from now after you forgot about it.
|
|
|
|
|
I suppose there is the possibility that a potentially exception throwing routine simply must be called in a destructor, and that logging or otherwise reporting the exception is not acceptable because allocating the necessary resources is potentailly unsafe, in which case your colleagues code would be quite acceptable. One might find a similar argument in a C++ library with an extern "C" interface.
Reporting or logging errors isn't always an option. Sometimes you just have to use other methods. Besides, further compounding the problem by obfuscating the errors is a suspect solution. Did you push your changes?
|
|
|
|
|
Time to point out that this is C# code.
|
|
|
|
|
...you mean to tell me that C# won't run on my doorbell?
|
|
|
|
|
I'd like to go a bit further, as a rule of thumb everytime time you go into the more general exception (as in your case) you MUST quit the application, of course you can log it but it's mandatory to quit if your program stayed in a compromised situation, otherwise you are able to inform your users about the problem (which should be very specific or maybe catch a more specific exception and solve it).
Many programmers do what your colleague did because they think it's not professional to quit that abruptly, wrong!, it's not professional not to accept the problem.
Carlos Alberto
|
|
|
|
|
Hah! You think that's bad?! Some time ago, I inherited some code which had about 70 data classes, each containing three or four methods along the lines of:
public override void CreateEntity()
{
try
{
base._database.ExecuteNonQuery(storedProcCommandWrapper);
int parameterValue = (int) storedProcCommandWrapper.GetParameterValue("@RETURN_VALUE");
if (parameterValue == 0)
{
throw new Exception("Create failed.");
}
}
catch (Exception exception)
{
exception.Message.ToString();
throw new Exception("Exception occured", exception);
}
}
Needless to say, all of the code which called these methods looked like:
try
{
customer.CreateEntity();
}
catch (Exception ex)
{
MessageBox.Show(ex.Message);
}
No logging. No access to the inner exception. No way to diagnose why one button was displaying the really helpful "Exception occured" error every time the user clicked it. 
"These people looked deep within my soul and assigned me a number based on the order in which I joined."
- Homer
|
|
|
|
|
If this is to teach him lesson then I agree with you, considering the fun.
But for deployment, it may cause serious problem.
|
|
|
|
|
Once, as a prank, a guy put an ASP.NET error message as data in the database (with the HTML formatting). Then he told another developer to look into a problem where in a HTML table on of the cells would display the ASP error message.
|
|
|
|
|
I could see him getting spoken to to correct his coding and you looking to explain to a new employer why you got sacked for sabataging the codebase.
|
|
|
|
|
FAIL.
Sorry to be a party pooper, but your evil intentions would not be fulfilled by that. You haven't overridden the GetType method. You've hidden it. And that means the compiler will only bind to the implementation if invoked on a reference of type TheClass. And it's unlikely anyone would call GetType on the thing if it's already declared to be TheClass (although the run-time type of course could be a descendant).
So, since GetType isn't virtual it'd still return "TheClass" and your colleague would go to the code and see that you've attempted to fool him. (If you guys have decent source control anyway.)
I do sympathize with your sentiments though. But frankly it is often the project managers who are at fault, because rather than foster a culture where mistakes are openly discussed, not for the purpose of punishment but for education, they tend to reward people who "are positive". In my experience that's often people who aren't competent enough to see the myriad things that are usually F-ed up in... well, any codebase I've come across.
|
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
