|
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.
|
|
|
|
|
Andrei Straut wrote: Yes, I'm a devious SOB, but hey, if they don't care, why should I?
If you didnt, then you and that person do not have/make/create any difference. Its just like watching a crime happening and ignoring it than atleast reporting.
|
|
|
|
|
hahaha lol... clever idea
============================================
The grass is always greener on the other side of the fence
|
|
|
|
|
So my boss is like a rabbit. He just loves his greens.
Unclear naming should stay unclear by commenting it (helper was never used, but that's beside the point).
Dim helper As Boolean Of course we could make clear comments in favor of properly naming our variables.
s.DoSomething()
p.DoSomethingElse The obvious also needs extra explanation.
i = nX + nY In case we ever want to compile our code into a beginners how-to-program book.
i = 10 We just can't have enough comments!
RefreshScreen Keep an active log inside your code, you never know when you need it (SVN alone isn't good enough)... (date, initials, ticket number).
The fun thing about this is that when a fix is larger than a few code files it does not have to be commented.
If it is only a few code files chances are this piece of commentary is copied to each of them, sometimes in multiple places in the same file.
CallSomeMethod Some of this guys methods are commented line by line with these kinds of comments... 
I tried to talk to him about it, but he's convinced code is unreadable and can never be understood easily by just reading it. Comments are absolutely necessary if you, or another programmer, ever want to quickly make changes to the code. This is the same guy who thinks making a seperate class for almost every method (and randomly make it non-shared/non-static or shared/static) makes code more clear and readable (and single responsibility principle is a bonus!) and who just doesn't use access modifiers out of laziness or indifference...
Of course he is my boss and has eight years of experience where I have only two (how often I had to hear that) so when I disagree with him I'm rude and inexperienced.
I guess someday I'll laugh about it 
It's an OO world.
public class Naerling : Lazy<Person>{
public void DoWork(){ throw new NotImplementedException(); }
}
|
|
|
|
|
Naerling wrote: Keep an active log inside your code
Ugh. I hate that. At my last job, they were all about that. Half a million lines of SQL, and none of it in version control (it was all nearly lost once, except I and another person had a personal backup).
Naerling wrote: I guess someday I'll laugh about it
When the company collapses and you find a new job with reasonable code practices.
|
|
|
|
|
AspDotNetDev wrote: When the company collapses and you find a new job with reasonable code practices.
Nah, they've been good to me and though I disagree with their code practices (and I disagree on almost everything with the guy who writes these comments) I don't want them to collapse... The funny thing is that the guy who writes these comments taught me to code! Luckily I surpassed my 'master' in every way (after about a year) 
Although he's still the better SQL Server guy.
It's an OO world.
public class Naerling : Lazy<Person>{
public void DoWork(){ throw new NotImplementedException(); }
}
|
|
|
|
|
Lack of decent version control is always a bad sign. 
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
(Really) Bad smell, think we should refactor!
|
|
|
|
|
I like to comment my code, but only where really necessary. I also use meaningful variable names and method names so that the code is largely self-commenting. It is rare to see someone who wants both pointless comments and meaningless names, fortunately.  If those things are wrong there are likely other bad practices and I would think about looking for a better place to work.
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
CIDev wrote: there are likely other bad practices
I found the same piece of code something like five times (EXACTLY the same). And then other pieces of code five times too. I found slightly different code even more often (don't make a function with input parameters, copy paste the code and change a word or two).
I found functions that were called ChangeX(ByVal x As Integer) that also changes Y and Z, ChangeY(ByVal y As Integer) that also changes X and Z etc. Worst part is, this guy coded them and still called ChangeX, ChangeY and ChangeZ one after the other.
Worst has still to come... This guy is technical director and pretends to be quality insurer (he is, but doesn't really insure anything).
He has told me I should not code MessageBox.Show("Hello"), I should've done:
Dim prompt As String = "Hello"
MessageBox.Show(prompt)
because... I don't know... He really doesn't understand I guess! If he would've coded it he wouldn't even have typed 'As String' because that's only inconvenient.
I've even seen him use switch statements for booleans! Somehow this guy doesn't like If statements...
And this guy is 'by the book' (really, if he sees something he doesn't like he always has a book that defends his position, it's freaky!)...
It's an OO world.
public class Naerling : Lazy<Person>{
public void DoWork(){ throw new NotImplementedException(); }
}
|
|
|
|
|
Yeah, that does sound pretty scary. It is expecially bad that he is a technical director. 
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
At our place we have a few of the "senior" apprentices staying with us in the dev team for a year. When I say "senior", they're about 19 in human years, but look about 8. I got to mentor one for a few weeks and threw him in at the deep end by giving him a MOSS WebPart to develop. For the most part, he's done an OK job and seems to be getting the hang of things, but still comes out with stuff like this:
int status = (int)(Status)Enum.Parse(typeof(Status), _statusBy.ToString());
this.dataSource.SelectParameters.Add("status", status.ToString());
Given that Status is an enum with integer values, this would actually suffice:
this.dataSource.SelectParameters.Add("status", ((int)_statusBy).ToString());
Bless.
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
