Coding Standards in the Workplace

Started by
35 comments, last by Big Sassy 22 years, 1 month ago
The problem with the comments is that they don''t tell you why.

I can quickly tell from the code:
theVideo->SaveScreenShot("menuscreenshot.bmp");
what it does. Your variable names and method name are decent.

But why are you capturing a screen-shot every time you start-up?
And why do you beat-pud for a second?
- The trade-off between price and quality does not exist in Japan. Rather, the idea that high quality brings on cost reduction is widely accepted.-- Tajima & Matsubara
Advertisement
Hello,
Once again good points about using exceptions. VC6.0 is one such compiler. I witnessed some others, but it has been several years. It can be fixed easy enough. For a while I used a nice template that fixed everything.

Oh no, I was not implying exceptions have too much overhead. Some people worry about checking a return status as slowing down their app.

I also liked your view on grouping error handling code. This leads to two cases
1) You put ''everything'' in a try/catch block, and have a big list of error handling at the bottom. This does indeed seperate the error code, but this is not good if you wish to still continue other steps when one fails.
2) So, to fix that, you start splitting up your try/catch blocks until it is at a level you like. At this point it is probably close to resemebling the Status block example. You call a function, continue, or handle the exception(s).

so in essence you have

  try{   Some fcall(...)} catch( case1 ){ ... }catch( case2  ){ BadError, throw another exception to not allow following code to execute. }try{  Other code... Should not execute if case2 from above}  


or


  if ( AllIsWell){   result = Some fCall()   if ( result == case1 ){ ... }   else if ( result == case2 ){ AllIsWell = false; }}if ( AllIsWell ){   Other code...Should not execute if case2 from above}  


Like you said, it comes down to what your prefer. As I said before, both styles have their places.

The rest of my comments dealt with Kylotan''s code and how using exceptions might lead to difficult to understand code, and what level of comments are really needed to understand a function. I was just refering to how he had only two returns, but does not seem to be handling any errors with his calls to theVideo. If the other calls to theVideo fail, how is he handling it? Yes indeed, anyone with the docs should be able to look it up. However, let us assume theVideo is throwing exceptions. As a maintainer, I must consult two different docs now to understand the intended behavior, which in this case is for Run NOT to catch theVideo''s exceptions. But the point is, by just looking at the code, or the comments, it is impossible to tell if theVideo throws exceptions, if I should catch them, or let my caller catch them.
Funny how everyone is discussing the ins and outs of my code, which was only there to back up certain points, not to act as a paragon of coding purity. But I may as well respond anyway.

quote:it may have been in the design to continue on anyway

It was. And I don''t care if someone else considers a failed blit to be exceptional, because in this project, I do not. I positively actively want the execution to continue. That is part of the specification.

quote:If Kylotan is throwing exceptions in theGame or theVideo, then there needs to be the catch. If he is catching in the calling function, then this particular example just increased in Maintenance complexity by forcing the maintainer to not only know the caller''s code, but the inside behavior of the called.
Specifying which exceptions are thrown by a function is a common issue and is a standard way of dealing with things, not an ''increase in maintenance complexity''. As it stands, my exceptions are dealt with in a low level handler, and I have a standard way of specifying exceptions. Essentially, you are criticising code you''ve never seen and never seen the design or specification for

quote:What I meant by not using error detection is because Klyotan was using a return value in some places, and not anything for your "theVideo" or "theGame" calls. Exceptions are good, but they are not meant to be used for every function call when a simple bool will do.

Better still, write functions than cannot fail. Not everything has to return an error value or throw an exception. Again, I refer anyone to Code Complete to see how to make functions that don''t break.

quote:and then you say it is not worth writing well structured code based on the fact there will not be any future revisions.

There''s nothing ill-structured about typing in 3 (three, count ''em) lines twice. There is a level of granularity beyond which you''re doing things a certain way for the sake of dogma rather than for any real benefit. Pixel class, anyone? The fact is, halting the program is not something I need to do more than twice, and it is not something that will be in the final compile. In this case, my forward-looking design overrides the need to write forward-looking code. I can concentrate on coding what I need rather than what I might need had I not designed it properly in the first place.

quote:It seems you prefer a high level comment with 5 lines of code VS a well named function (in this case drawBackround and Wait).

No, I prefer knowing in advance which functions I need and coding those, rather than going to the trouble of coding functions that are just going to get removed anyway. Having separate functions for every individual bit of functionality gives you a wide interface which is harder to remember and to maintain. You don''t need DrawBackground. It does nothing that DrawImage doesn''t do. There is no distinction between a background image and any other image, so to pretend there''s a distinction between them is a bad idea. The DrawBackground function is a bad idea. Interfaces should be minimal and complete.

quote:But why are you capturing a screen-shot every time you start-up?
And why do you beat-pud for a second?

I take a screenshot because I want to. This code is being Way overanalysed It was posted as an example of how comments can not go out of date if they are written a certain way. It wasn''t meant to be a perfect bit of code. I don''t write perfect code, nor have I claimed to. The waiting for a second is so that you get a chance to see the screen before it closes down.

quote:If the other calls to theVideo fail, how is he handling it?

They don''t fail. Next question?

quote:However, let us assume theVideo is throwing exceptions. As a maintainer, I must consult two different docs now to understand the intended behavior, which in this case is for Run NOT to catch theVideo''s exceptions.

As a maintainer, if you use a class, you have to read up on that class! You can''t expect every instance of usage of that class to contain details of every exception it might throw. I don''t expect the return values of a standard library function to be regurgitated in a comment every time I use it. Who catches exceptions and how they are caught is not something you can localise. You need to have a (sub-)system-wide strategy for this, so expecting to be able to tell everything from one function taken in isolation (which I remind you was to demonstrate an utterly different and unrelated point) is unreasonable.

[ MSVC Fixes | STL | SDL | Game AI | Sockets | C++ Faq Lite | Boost ]
I work at a steel company in the midwest and we have no formal commenting or coding standard. Generally, things written in PowerBuilder (don''t touch it with a 20 yard pole) have a great deal of commenting to the right of the line with blocks at the beginning of functions. Some more complicated objects will have commenting on every line. This is mainly due to the buffet-style, work on what needs-a-fixin project assignments here.

There are only 2 VB programmers here. I comment right of line on most code blocks with descriptive blocks above function calls. The other guy comments so he can understand what he did but not much more.

In C++, well, I''m pretty much the only guy here that writes anything in C++ so I comment however the hell I feel like. Usually, top of page blocks, per-function blocks, and on their own line in code.

Sadly, the ASP code has almost no comments.

We have no notation guidelines... They primarily write in PB which is so plain-english, notation would probably make it harder to read.
Kylotan,
Anony and I (at least I) were more going after coding style than picking on your individual piece of code, which is the thread topic. Your original function was only about 20 lines to begin with, so any real changes automatically fall into the nit-pick catagory.
Yes, I agree, 3 lines of code may not justify creating a function. That is up to you. The main talk was about use of exceptions and stucture of code and not your test routine.
I especially was enjoying talking about the pro''s and con''s of using a mostly exception handling model VS the more C approach, as Anony put it, using return values for control.
However, I am worried about your comment "It will not fail". Never assume, especially if it is the one touching DX or another GDI. Sure, YOUR code may not fail, but what about something you are calling? I have seen that high level exception handling technique you are talking about, once such case is in Adrian Perez''s book I believe.
quote:Original post by Taulin
However, I am worried about your comment "It will not fail". Never assume, especially if it is the one touching DX or another GDI. Sure, YOUR code may not fail, but what about something you are calling?

This is no different from setting a ''nothrow'' specification on a function. What I meant by "it will not fail" is that it will not throw an exception, and it does not need to signal an error. If any of the APIs I call throw exceptions, I handle them. If those APIs return error codes, I process them where necessary. Any other errors that occur during the execution of the function are beyond my program''s scope. If DirectX somehow throws an exception when it shouldn''t, that is no different to a power surge flipping some bits in the RAM somewhere causing a crash. I handle all reasonable and deterministic code paths. I even make a vague attempt to handle several unreasonable code paths with the catch(...) in my main() function. But on the whole, I treat the computer as a deterministic Turing machine, which is what it is intended to be. Some people disagree with this and program everything defensively as if everything could go wrong, and I disagree with that approach too. If you''re going to start assuming that 2+2 can equal 5 under certain conditions, then there''s no point programming.

[ MSVC Fixes | STL | SDL | Game AI | Sockets | C++ Faq Lite | Boost ]
I was just commenting on your "They can not fail" comment. Failures in this case could be anything, including those that would make you need to shut down your application even after theVideo tries its best to handle any errors. You would not just want the whole thing to crash, you would most likely want to try to save any data, and shut down the best you can, especially if you are running in a Win-9X environment. It seems you are catching these exceptions in your main, which I would imagine would cause it to turn around and tell the rest of the sytem to shut down. However, at this point, I am starting to assume your application is doing way more than it probably is, but once again, I never inteded to talk about your code directly. I always raised my points as assumptions to continue this conversation as a case by case debate.
I agree that it is pointless to start worrying about everything, but when it does hit the fan, its always best to back out as smoothly as possible, and not just consider it as a power surge crash.

This topic is closed to new replies.

Advertisement