I’ve seen this now twice in about 30 mins, and its bugging me.
One of the developers i work with is writing code like this:
string postingUrl = CmsHttpContext.Current.Posting.Url.ToLower(CultureInfo.InvariantCulture);
Whats bugs me with this code is that it shows no understanding or care for defensive programming.
Q1: Why do you assume that CmsHttpContext.Current is safe? Sure, within ASP.NET the framework creates a new Context for you upon request, and (in this case) Microsoft CMS wraps the HttpContext and guarantees you a copy of a context for you. Under these known conditions, the CmsHttpContext.Current is safe.
Now that’s been chained onto Posting, which throws the exception. Why assume that you’ve hit a posting? Why assume that there would be a posting at all? This kind of lack of thinking just demonstrates lazy programming, to me.
*grumble*
It’s a pitty you are not telling this face-to-face.
Besides I fixed this bug and do really know what deffencive programming is.
Regards,
Evgeny
I’m actually reading your comment for the first time, and i’m glad i did talk about it to you face-to-face the very next day. Wrapping the exception in a try..catch isn’t the right- fix because:
1. It creates unncessary work for the app when it has to unwind the stack and process the exception
2. It will swallow any valid exception (eg: database offline) rather than report it.
Being defensive doesn’t mean putting try..catch everywhere or checking every object for existence, but rather one part of it is in determining points of failure and covering those.
It’s never perfect.
You right, I’m wrong. I have no idea what defensive programming is.
I will not put try-catch blocks everywhere and check every object for existence.
I will never create unnecessary work for the application.
I will never be perfect. I will never argue against your opinion and dispute your encyclopedic knowledge.
I will never be a lazy programmer (whatever that means) and assume anything.
Amen!
Now seriously.
Thanks for talking to me face-to-face. I really appreciate that.
As I don’t have my own blog I’m writing my thoughts here if you comfortable with that.
Telling about myself and why I’m responding to your post and comment. I’m not teaching anyone just expressing my own opinion and what I’m expecting from myself and you can expect from me:
1. I would never criticize a person I know personally without telling him.
2. I would never make assumptions on people’s level of coding (or whatever) on one line of code/one sentence/one word etc and express that in the way “there is a man who doing blah blahâ€. Everyone has the name. And first assumptions are usually wrong.
3. I know that my code is not perfect and I don’t usually have time and/or budget for “optimizationâ€. But I’m doing my best to make it better ïŠ
4. I know that my code has bugs, the only thing I’m doing is gradually increasing its quality
5. When I’m criticizing anyone’s work I will state my point in addition to critics. In other words I’m expecting to get some new knowledge and understanding why I’m possibly wrong than just get a portion of critics (which is meaningless in that case and initiates unneeded arguing)
6. I’m always respecting other people’s knowledge and decisions and extensively think before criticizing them. I’m respecting your opinion and it’s quite possible you are right (at least at some extent for sure)
7. I will never tell “this code is bad or wrong†as soon as it’s obvious or this code would do something I define as bad or wrong
8. I believe making mistakes is the main way you can move forward
9. I believe laziness is really a good thing for a developer (in terms of code re-usage, making less mistakes etc so you would not spend your time fixing them or doing the same things thousand times)
10. I believe computers are really fast and there is no need in micro-optimization
11. I believe 95% of performance bottlenecks are caused by 5% of code
12. I’m writing these lines only because I have free time and I’m not feeling comfortable with your post (and believe that this post is about me). In other words you wouldn’t get this response as I don’t really care what anyone is talking/writing/thinking about my person and I’m not wishing to be good for everyone (my family is the only exception). I’m being paid for the work I’m doing and will do that work the best I can in terms of business and clients needs and even better. Arguing is just wasting my and your time. I will not argue as soon as I’m not feeling it’s useful for either side (mine is preferable :). I’m feeling it’s becoming pointless.
Some words about developing.
13. Being defensive doesn’t mean putting try-catch blocks everywhere. (Looks the same because I’m fully agree). Making assumption on 1 line of code is even worse than making assumption on object state in the same line of code.
14. Fixing a problem means the problem doesn’t exist anymore. All other are variations. There could be different ways to achieve the same goal and the “lazy†way is not always the worst way.
15. There are no “right” or “wrong” fixes to the problem as soon the problem is eliminated and the business goal is achieved.
16. Putting try-except block in the minor part of an application which will halt on exception in less than 0.1% of calls is not really a bad thing (not doing this means that in case of exception the whole application will die of this minor functionality’ failure). With such minor impact the stack lookup will take tiny amount of time. I call it over-optimization. Besides you still going to receive major exceptions such as DB connectivity etc. YAGNI is the main concept I’m using when making such decisions.
17. You can check for object existence in the minor app functionality but what the sense in that if code is tested? If the whole block failing (this behavior is not tested) just put it in try-catch block in case the application will continue work without it correctly, why do you need to over-complicate that?
That simple from my side.
Without trying to continue what would become an unproductive and harmful debate, there are a couple of points which I think are quite pertinent to this issue, and they come from what you’ve just said:
1. You are right in that I shouldn’t have put this up online BEFORE talking to you about it. That’s something i’ve learnt from this experience, and I do apologise for that.
2. I will never identify/name anyone on my site because there is no benefit in publicly naming and shaming. Everyone has a name and identity, but noone deserves to be identified and smeared in public, so i choose not to name people.
3. By “lazy” i’m not talking about when people think of clever ways to reduce effort, but when people don’t think of the full-picture and the effects of the code they are writing.
4. When you say that the try..catch affects 0.1% of the application, you’re not thinking of the full-picture. That try..catch() gets triggered on every 40x/50x error page, and from the past 3 weeks of performance problems, you’d know that the 500 Server error page has been by far the WORST problem we faced. If it were compounded by a bug like this, we would be putting the webserver under intense, unrequired load.
5. You mention testing a number of times, however I’m currently in the process of fixing a bug in one of the modules you’ve written, and I cant find unit-tests around the majority of the code. I don’t want to publicly detail any issues, but if you’re interested in knowing what I feel is critical to have been tested (and doesn’t appear to have been), email me.
What i was talking about in my previous posts was not a very heavy-optimisation. I’m not talking about YAGNI. I’m talking about a decision on which way to implement a feature, and then the corresponding fix which was made.
I’m not ever 100% right, and I would hope i never am – i completely agree that you only learn from your mistakes so i’m happy to show you where I think some of this code can be made better, and we can discuss it at length; in person.
-xerx
Thanks, really appreciate your apologies.
Think we can productively increase our expertise in the field together.
TDD was never one of my strong skills (well, some critical apps I’ve written without TDD are still working for more than 10 years 🙂 and I’m only beginning to realize the power and importance of this approach. Sorry for not using it everywhere and 100%.
As I said it’s better to learn from each other than to shame each other as it’s a kind of “family†business.
That’s why I don’t have a blog.