Want to read Slashdot from your mobile device? Point it at m.slashdot.org and keep reading!

 



Forgot your password?
typodupeerror
×
Programming IT Technology

Best and Worst Coding Standards? 956

An anonymous reader writes "If you've been hired by a serious software development house, chances are one of your early familiarization tasks was to read company guidelines on coding standards and practices. You've probably been given some basic guidelines, such as gotos being off limits except in specific circumstances, or that code should be indented with tabs rather than spaces, or vice versa. Perhaps you've had some more exotic or less intuitive practices as well; maybe continue or multiple return statements were off-limits. What standards have you found worked well in practice, increasing code readability and maintainability? Which only looked good on paper?"
This discussion has been archived. No new comments can be posted.

Best and Worst Coding Standards?

Comments Filter:
  • by AndGodSed ( 968378 ) on Sunday July 20, 2008 @09:24AM (#24261601) Homepage Journal

    Sound an awful lot like coding in C... no bad coding practice needed...

    • by laejoh ( 648921 ) on Sunday July 20, 2008 @10:48AM (#24262235)

      Now that we're talking about 'languages that invite bad coding practices'... Well, one of the best programming books I've read is 'Perl Best Practices'. Not only does it list out best practices but it tries to explain (well I might add) why you should code a certain way and why other ways aren't good to follow.

      One of the habits I picked up from 'Perl Best Practices is:

      if (condition) {
      }
      else {
      }

      instead of:

      if (condition) {
      } else {
      }

      The else tends to get 'lost' when just following the closing bracket.

      • by Maxmin ( 921568 ) on Sunday July 20, 2008 @11:52AM (#24262791)

        "no NO!! Must not put elses near braces, my precious!" - Larry Wall

        I, for one, have never understood Larry's War on Cuddled Elses. It's almost symptomatic of OCD.

        Besides, how is the "else" getting "lost?" I mean, it's only two characters from the left margin! Saves lines too.

        Maybe it's that I prefer reading source that is not so vertically spread out. The more code and logic on the screen, the better. Density factor.

        • by Doctor Crumb ( 737936 ) on Sunday July 20, 2008 @03:33PM (#24264935) Homepage

          Density is the opposite of readable and maintainable. One of the main aspects of maintainable code is being able to find and change a single line of code quickly and without having to worry about breaking other nearby lines. Having more code per line means it takes that little bit longer to find and is that little bit riskier to change the one line of code. "Lost" in this case is only a minor delay, but when you add that delay up across several thousand bugs (i.e. any project in the Real World), it means you are wasting your time tracking down bugs in dense code rather than adding new features or working on other projects.

          Pop quiz: find and remove the bracketed clause in the above paragraph. Then think about how much faster you would have done that if it had been on its own line. Then think about how much faster you could have done that to 100 different paragraphs. It may not make a difference on the projects you work on, but in something the size of a perl interpreter or a web browser, it makes a huge difference.

          • by telbij ( 465356 ) on Sunday July 20, 2008 @05:25PM (#24265789)

            Density is the opposite of readable and maintainable.

            Bollocks. It's a tradeoff just like every other debate in the programming world. Sure, Perl gives you the ability to put way to much code on a single line. But the opposite problem of putting loads of white space all over the place is almost as bad.

            The more you spread out the code, the more you have to scroll. White space is valuable when it means something, like to separate discrete tasks within a long function. But the whole


            }
            else {

            thing is just a waste of space. It's one line less of code I can see. I visually parse } else { instantaneously. Similarly, some compound expressions or chained method calls make perfect sense. The right place to break out multiple lines depends on the reader's own cognitive abilities and familiarity with the symbols being manipulated.

            Otherwise
            writing
            like
            this
            would
            be
            much
            easier
            to
            read

          • by Maxmin ( 921568 ) on Sunday July 20, 2008 @06:21PM (#24266245)

            My dear Doctor, readability metrics boil down to personal tastes - subjective, in other words. While perhaps what you choose is even the preference of the majority of coders, it's not mine.

            Advocating for braces and the negatory conjunctive "else" on the same line is not the same as "having more code per line," e.g. more than one statement per line.

            In the responses to the OP, we've if/else on three lines-

            if (...) {
            ...
            } else {
            ...
            }

            -and six lines-

            if (...)
            {
            ...
            }
            else
            {
            ...
            }

            Whilst reading six lines is not a problem for me, I prefer the three line variant, as it means less scrolling over slow ssh connections. Thank goodness we have automagic reformatting IDEs for those who won't accept other formats.

            However -- it is a sorry state of affairs that on /. the replies to an enquiry about "coding standards" end up focusing on code formatting... I'd much rather have been debating architectural design patterns as the response to "coding standards."

            Positions on design patterns, over the last few years, appear to have accreted into two clusters, those for and those against. I am one of those in the "for" camp, where learning the whys and wherefores of a particular set of data structures and classes, and behaviors arising from said structures, determine architecture.

            Those "against" appear, to my reading, to be willing to forgo such learning and accept whatever baked-in design patterns the platform's designers chose.

            Now, on the one hand, I accept that that's the case, as there is an observable stratification of programming ability existing in the world of software developers. One most go with one's strengths, and not everyone is suited to solving the issues architecture.

            On the other hand, if a developer is so inclined, there is still plenty of latitude available when structuring applications.

            Finally, there appears to have been a rise in the strongly anti-design patterns camp - the learning and applying thereof, that is. Most particularly, the anti-Java, pro-Ruby/RoR camp, where seemingly one must accept the baked-in design patterns chosen by the platform's architect, without variation.

            A direct descendent of that camp, the adherents to the prototype.js and scriptaculous libraries, accept the original author's patterns to the point where performance deficits due to overuse of lambda functions are not only accepted, they are ignored.

            That, IMNSHO, is sad comment on the state of software development. Productivity over performance is an awkward choice, to say the least.

      • by xjimhb ( 234034 ) on Sunday July 20, 2008 @11:54AM (#24262809) Homepage

        Why does everybody do it that way? That is, with the opening paren on the "if" line? I have always found that difficult to read. Why not

        if (something)
        {
            stuff
        }
        else
        {
            other stuff
        }

        or maybe even

        if (something)
            {
                stuff
            }
        else
            {
                other stuff
            }

        This last has always seemed to me to be the most readable, most obvious way to write the code. Can anyone explain why it is not used? (other than some well-known guru prefers the other?)

        • by Zwicky ( 702757 ) on Sunday July 20, 2008 @12:11PM (#24262971)

          From a personal perspective that happens to tie in with the coding practices at my last company:

          The second example (GNU style) I have found to be quite cumbersome in writing, unless tabs are set to 2 with braces indented once and content twice (company mandated four with one indent for content in the block), in which case I would be frustrated with the extra keypresses involved.

          The first example (Allman style) I used to use until I moved over to Kernighan-Ritchie style (opening brace on same line as control statement, with functions (and classes in OOP languages) braces the exception; these are written in Allman style). This allows me to scrunch more onto the screen vertically.

          FWIW I never liked the '} else {' style of elses but at the same time, I never found it difficult to read so it was never a real issue. It makes sense to me to have the else begin at the same column as the if to which it belongs.

          This [wikipedia.org] may be of interest to you.

        • by CastrTroy ( 595695 ) on Sunday July 20, 2008 @12:22PM (#24263073)
          I like the braces on separate lines also. Makes things a lot more readable. Another good idea is to always put the braces in, even when you are only writing a single line. That way when somebody goes to put more code in the if statement, they don't have to remember to add them.
      • by bperkins ( 12056 ) on Sunday July 20, 2008 @12:28PM (#24263147) Homepage Journal

        So we have a shop that has a whole lot of perl code and they're sent around this book as well as run perlcritic on our checked in code (which pretty much everyone ignores).

        In my couple of years there I've learned a few things.

        1) No one can agree on coding standards

        2) What people can agree on is so watered down that it's not very useful.

        3) The stuff that really causes headaches isn't bad style, it's general insanity. Hardcoded constants and poorly thought out ad-hoc parsers and general brain damage causes a million times more problems than just about anything anyone can describe in a standard.

        That said, in my experience the one thing that almost aways saves me time for anything larger than a couple of lines is to use "use strict."

      • by kramulous ( 977841 ) * on Sunday July 20, 2008 @05:07PM (#24265659)
        I don't know where I picked it up (Java ??!) but I pretty much always use:

        if (condition)
        {
        // something
        }
        else
        {
        // something else
        } // End of X branching

        I find that tracing parenthesis is a lot easier.

  • Space Usage (Score:5, Interesting)

    by Nerdfest ( 867930 ) on Sunday July 20, 2008 @09:30AM (#24261647)
    I've worked where we were supplied a full IDE and a 17" CRT, and the coding standard forced so much white space vertically that you had to basically remember all the code.
    • Re:Space Usage (Score:5, Informative)

      by LordOfLead ( 1121501 ) on Sunday July 20, 2008 @11:41AM (#24262719) Homepage

      The Linus says:

      "If you need more than 3 levels of indentation, you're screwed anyway, and should fix your program."

      from http://en.wikiquote.org/wiki/Linus_Torvalds [wikiquote.org]

  • braces (Score:3, Interesting)

    by __aapbzv4610 ( 411560 ) on Sunday July 20, 2008 @09:30AM (#24261653)

    I can't stand seeing the closing brace of an if statement sharing a line with an else, like so:

    if( condition ) {
        statement1;
    } else {
        statement2;
    }

    • Re:braces (Score:5, Interesting)

      by Dionysus ( 12737 ) on Sunday July 20, 2008 @09:43AM (#24261717) Homepage

      If Kernighan, Ritchie [wikipedia.org], and Torvalds [linux.no] does it like that, who am I to do differently.

    • Re:braces (Score:4, Interesting)

      by tolomea ( 1026104 ) on Sunday July 20, 2008 @09:46AM (#24261741)

      I really don't get this obsession people have with putting braces on separate lines.

      Block scoping is perfectly well indicated by indentation and blank lines are valuable for dividing functional blocks of code.

      When you go and put all the braces on separate lines it totally kills the visual effect of the actually empty lines. Then you'll have to go and start using multiple blank lines for separating things and before long half the screen will be empty and mostly empty lines.

      Hmmm perhaps that's it, maybe this is a scheme for people who don't like looking at the code.

      • Re: (Score:3, Interesting)

        by TheRaven64 ( 641858 )
        Maybe the way you code, you never have a line that is so long that it needs to be broken. In most C code I've seen (and all C++ or Java code) a lot of lines exceed the 80-column width that is a common maximum for coding conventions. You then can't clearly distinguish between the start of a block and a wrapped line without clearly inspecting the indenting. This is even more true in Objective-C, where long methods are often written with one parameter on a line.
    • Re:braces (Score:5, Insightful)

      by heffrey ( 229704 ) on Sunday July 20, 2008 @09:47AM (#24261749)

      It doesn't really matter what you do, so long as everyone on the team does the same thing.

    • Re:braces (Score:5, Interesting)

      by Dachannien ( 617929 ) on Sunday July 20, 2008 @09:48AM (#24261761)

      I don't like seeing opening braces sharing a line with anything else at all, unless the block gets closed on the same line.


      if(something)
          {
          do_something();
          }
      else
          {
          do_something_else();
          }

      Yeah, it takes a bit more space, but I find it a lot easier to match blocks up when the braces are indented the same amount.

    • GNU Indent (Score:4, Insightful)

      by Jim Hall ( 2985 ) on Sunday July 20, 2008 @10:04AM (#24261861) Homepage

      At my first industry job (internship) I quickly realized their coding standards were very different from mine. So I spent the first 2 hours after lunch on day 1 with GNU Indent, working up a script that would convert my code into the company's coding standard for indentation.

      Let the tools do the work for you. Just don't forget to run 'indent' before you check in your code.

    • Re:braces (Score:4, Insightful)

      by harry666t ( 1062422 ) <harry666t@DEBIANgmail.com minus distro> on Sunday July 20, 2008 @10:24AM (#24262023)
      Braces, braces, braces... I code in Python, which seems to be the only computer language that got the braces right:

      if condition:
          statement1
      else:
          statement2

      That's it! No braces at all.

      Well, maybe except in dictionaries:
      mydict = {"foo": someObject,
                "bar": 42,
                }
      • Re:braces (Score:5, Informative)

        by Haeleth ( 414428 ) on Sunday July 20, 2008 @11:21AM (#24262559) Journal

        Like many Python evangelists, you seem to have a remarkably limited experience of computer languages.

        Here's a language with no braces:

        IF condition THEN
            statement1
        ELSE
            statement2
        ENDIF

        Here's another:

        (cond (condition statement1)
              (t statement2))

        Here's another:

        if condition then begin
            statement1;
        end else begin
            statement2;
        end;

        Here's another:

        if condition then
          value1
        else
          value2

        or, more extensibly,

        match condition with
          | true -> value1
          | false -> value2

        And the list goes on. Maybe you should try learning some other languages. Broaden your mind a bit. There's a lot out there that isn't Python or C/C++/Java. Some of it is quite interesting.

    • Re: (Score:3, Informative)

      by mikael ( 484 )

      That is the Indian Hill C Coding Standard [psu.edu]. It is almost mandatory to learn for certain areas of computer programming such as device driver development and applications programming. Mainly because it is the first documented coding standard that came out and was used by universities and corporations.

      I've known some companies to make it a priority that all programmers used this standard when their greatest threat to survival was keeping up technologically with their competitors.

  • by PsyQo ( 1020321 ) on Sunday July 20, 2008 @09:34AM (#24261671)
    I've always found the Joint Strike Fighter's coding standards document an interesting read. It is available from Bjarne Stroustrup's website [att.com] (pdf)
  • Serious != good (Score:4, Interesting)

    by Exanon ( 1277926 ) on Sunday July 20, 2008 @09:36AM (#24261681)
    This sounds like a fairytale but I work for a very large IT firm which is very well known. Serious company doesn't mean good however.
    In certain files (not all apparentely) all constant variables have to be declared globally. We are talking C++ here.

    Think what you want, but I don't like it. The reason for the variables placements are so "that they will be easy to find".
  • It's easy (Score:5, Funny)

    by krkhan ( 1071096 ) on Sunday July 20, 2008 @09:40AM (#24261707) Homepage
    First off, I'd suggest printing out a copy of the /. comments, and NOT read it. Burn them, it's a great symbolic gesture.
  • My new standard (Score:5, Interesting)

    by thogard ( 43403 ) on Sunday July 20, 2008 @09:44AM (#24261729) Homepage

    My new standard comes from a 1950's comp sci book.

    "Programs consists of input, output, processing and storage."

    Lose focus of that and the project will be late, over budget and most likely broken in ways no one will understand for years.

  • by Lucas.Langa ( 922843 ) on Sunday July 20, 2008 @09:47AM (#24261751) Homepage

    If you are using your computer right, it does not only enable you to do things, it does the boring things for you, automatically.

    Checkstyle is one of the tools in a company toolkit that is often overlooked but in fact VERY handy. It enables you to define a ruleset for your source code, finding stuff which is incompatible with the coding practice in your company/team/project/whatever. Moreover, you can stick it into Eclipse using the free Eclipse-CS plugin, so it will automagically mark the places which need to be change. Last but not least, you can put Checkstyle as an Ant task in your building environment (and in your continous integration toolkit) so commited code that does not conform certain standards does not build.

    As for the rules themselves, we've found these to be the most successful:

    • limit the length of the method to be visible on one 1280x1024 screen; if it's longer it's probably handy to extract smaller methods from it - which will be far more easy to read
    • similarly: set a file length limit (e.g. 1000 lines should be enough for everybody)
    • an upper limit on the allowed number of method parameters seems to be a good idea as well
    • ensure that new code is commented by marking structures which could be javadoced but aren't
    • any naming and formatting convention is better than no convention; Checkstyle can use regular expressions to validate type, variable and other names. It can also check for whitespace constraints, new lines, etc.
    • avoid star, redundant and unused imports
    • enforce or forbid the usage of the tabulator character to have all code clean no matter where you open it
    • warn on redundant modifiers
    • enforce the usage of braces everywhere (e.g. disallow if (something) action; ): no misformatting will hide a trivial but dangerous bug
    • a major one: Checkstyle can warn if it discovers a typical coding problem (of course this has some false positives but better to be on the safe side): double checked locking, lack of hashcode when overriding equals, switch fallthroughs, illegal catches and throws, lack of super.clone() or super.finalize(), etc.
    • you can also constraint the number of returns from methods (we found it to be very useful to set this to 1, using a result variable everywhere else - it's far easier to get hold of the code flow then)
    • we also constraint the if depth and boolean expression complexity to manage the cyclomatic complexity - also for easier reading
    • it's useful to make Eclipse clean up your code on every save: to add "final" where it can, to fix imports, format the code to the specified form, etc.

    Of course, we let developers to add suppresions for the 1% of false positives. In fact, there are very few suppresion rules set.

  • developer buy-in (Score:5, Insightful)

    by Dionysus ( 12737 ) on Sunday July 20, 2008 @09:48AM (#24261765) Homepage

    Without developer buy-in, whatever coding standards you come up with will be useless. IOW, ask your developers to create the standard together.

  • precise spacing (Score:4, Interesting)

    by griffjon ( 14945 ) <GriffJon&gmail,com> on Sunday July 20, 2008 @09:59AM (#24261821) Homepage Journal

    One of my friends worked at a place where you'd have to insert whitespace to place certain elements (variables, evals, etc.) to begin at a specific col in the code within every line; in addition to standard indentation of the line. At one level, I see the concept, but seriously - highlighting and search is made to solve the same problem there.

    He left that job quickly.

  • by hucke ( 55628 ) * on Sunday July 20, 2008 @10:14AM (#24261927) Homepage

    I worked for a company that was destroyed by a bad coding standard.

    This was a small company, that, back in '96, was awarded the contract for a POS application for a regional store chain, with back-office servers that would be updated nightly by modem.

    The guys who ran the company weren't programmers (though one of them knew enough to be dangerous); they were technical salesmen. They were also big fans of Microsoft, with "MVP" plaques on the walls, and every employee except me having Microsoft certs.

    I worked for them part-time while also working for another company. I advocated Unix (mostly BSDI and SunOS at the time), and always argued with them about why Unix was better (technical superiority vs. potential for big profits).

    When their big project was well underway, they brought me in to do the communications part of it, where the POS terminals would contact one of several servers by modem each night ("why not just ethernet them together, get a dialup PPP connection, and use IP? the interface is so much more reliable..." Request denied).

    The app was Visual Basic, with third-party "custom controls" for things like talking to modems. My part went fairly smoothly, and I was eventually asked to help out with the main application, which was suffering from unexplained crashes. When I looked at the code, I found something... strange.

    For error handling, they had elected to use a program called "VB Rig" (the name came from the rigging used on sailing ships, which prevents a sailor from falling to his death. Sometimes.) What this program did was to examine the source code, and then add error handling boilerplate at the start and end of each and every function. It inserted the exact same error handling code into every function.

    Because the error handler had to be all purpose, it was about 20 lines of code per function - sometimes much larger than the regular part of the function. And, worse, because it was the same for every function, and it made use of the same variable names, that meant either every variable had to be global, or you'd have to declare the ten or so standard variable names at the start of every function (they opted for the "everything is global" approach).

    Which led to things like this (forgive the syntax errors, it's been years since I've touched VB):

    On Error goto my_data_file_read_function_VBRIG_TRAP

    open MyDataFile for writing ...
    goto my_data_file_read_function_VBRIG_CLEANUP

    my_data_file_read_function_VBRIG_TRAP:
    on error 101 'Permission Denied
    delete MyDataFile
    resume
    on error 102 'File Not Found
    MessageBox 'Cannot read ' + MyConfigFile
    resume
    my_data_file_read_function_VBRIG_CLEANUP:
    blah blah
    my_data_file_read_function = SUCCESS ' return

    As you see, the error handling code - which had to be exactly the same for every function - made use of global variables (names like DataFile1, MyFile1, UserName, etc.) to figure out what to do for each error. That meant, that if there was any possibility you might have a "File Not Found", you had to expect the filename where that might happen to be in a particular global variable - say, MyFile1 - and hope that the calling function wasn't using that name too, for the same reasons.

    Naturally, files were being created and deleted at random, and the programmers often spent hours on the phone with the customer trying to figure out why the Access database had disappeared *again*.

    I asked if we could just write the error handling by hand, and use appropriate local variables; or take the standard VBRig error handling and trim out the lines that weren't relevant for a particular function (as subsequent VBRig runs wouldn't touch its code region if it saw that it had been customized).

    Request Denied. "This is our coding standard. We carefully reviewed the options before making the decision to use t

    • by Splab ( 574204 ) on Sunday July 20, 2008 @10:24AM (#24262025)

      I love abbreviations, not being native English speaker I used to think POS was "piece of shit" since its usually being used when talking about software (failed software).

      (Or the always common "IANAL", well good for you buddy, but we are talking about legal issues here - the arse pounding is for when you get behind bars)

  • Correct focus (Score:5, Insightful)

    by QuietLagoon ( 813062 ) on Sunday July 20, 2008 @10:15AM (#24261937)
    What standards have you found worked well in practice, increasing code readability and maintainability?

    .

    Coding guidelines are typically justified because, as it goes, most of the time is spent fixing bugs in existing code than writing new code. The guidelines are needed because it helps others to come up to speed quickly while they try to figure out the code in which they have to fix the bug(s).

    I think that is the wrong focus, as it tends to reinforce incorrect behavior, i.e., the writing of buggy code.

    Coding guidelines should focus instead on the techniques that help reduce the number of bugs in code. How is that done? It takes someone (typically a senior person) looking at the the bugs that have been found in the code, categorizing their cause, devising a way to prevent those bugs from occurring, then putting that into the guidelines.

    Keep the focus of the guidelines where it should be: to increase the quality of the software.

    • Re: (Score:3, Insightful)

      by pz ( 113803 )

      I wholly agree, and find that Conway's "Perl Best Practices" does an excellent job of just this, for people who are writing Perl. In the book, Conway goes through a number of different options and describes why they're just bad ideas, arguing mostly from a maintenance and clarity perspective, and cogently suggests superior options.

  • Embedded Coding (Score:4, Interesting)

    by JPEWdev ( 770760 ) on Sunday July 20, 2008 @10:17AM (#24261945)
    I write code for embedded systems where there are many hundreds of C files that all need to compile together to form a single executable (everything shares a single address space). Since there are alot of modules working together, the most useful thing is the usage of prefixes for modules components. For example, all of the database public methods, defines, etc. are prefixed with a DB_ and all the private ones are prefixed with a db_. Granted, it is up to the programmer to enforce these restrictions, but it is nice to be able to tell exactly where a function comes from when reading through some else's code.

    On the strange side is the omission of vowels on functions and varible names to save text space (it's not required, but should be consistent for similarily names objects). It sounds weird, but is still quite readable.

  • by khendron ( 225184 ) on Sunday July 20, 2008 @10:30AM (#24262069) Homepage

    I've come across, been subject to, and written many coding standards during my career and have come to the conclusion that coding standards are, for the most part, useless.

    It is much better to let coders program to whatever style they are most comfortable with. Forcing specific bracing and indentation styles just leads to ill-will from most coders and creates a bureaucratic overhead that is more more trouble than it is worth. As long as the style is consistent within a single file, most programmers will have no trouble following it and have no trouble maintaining the code.

    The only standard I think is worthwhile, when coding API libraries and such, is a naming standard. For example, a coder should not have to guess whether the method to get a property value is getProperty() or property_get() or propertyValueOf() or whatever. I don't care what the naming standard is, as long as it is consistent across the API.

  • by Shados ( 741919 ) on Sunday July 20, 2008 @10:35AM (#24262113)

    Each language or environments have their own features, and require different standards. One of the big things is that hopping from one project/company to the other should be intuitive (something thats basically forfeited in environments such as C++, and accepted as to not be possible, more or less) When the language is mainly controled or orchestrated by a single entity (Sun for Java, Microsoft for .NET, etc), people should set aside their own opinion and stick to the main guidelines (and complete them for areas where the main design guidelines do not cover).

    So for example, in .NET, stick FxCop or Code Analysis on, disable the rules that aren't relevant to your company (ie: localization rules on an app that doesn't require them), and stick to that. C++/VB6/Java/Smalltalk conventions have no place in there, so leave em out.

    Same holds true for any other environment. Don't use VB6 conventions in Java/C++ (I know the thought alone seems mind boggling, but I've seen it countless of times....ugh!), and so on.

    The biggest issue with conventions is just that: people take conventions that are specific to one language/environment, and don't realise that they are, so they port them everywhere else, so you have a program in language X that literally looks like if it was written in language Y (and takes twice as much code, is twice as buggy, is half as readable...)

  • Comment removed (Score:5, Interesting)

    by account_deleted ( 4530225 ) on Sunday July 20, 2008 @10:38AM (#24262131)
    Comment removed based on user account deletion
  • by Pedrito ( 94783 ) on Sunday July 20, 2008 @10:40AM (#24262149)

    I work for a major software vendor. The particular group I work in wrote the application framework for a suite of apps. Our code is mostly quite nice. There were about 20 people working on it during development and there are a few pieces that are crap, but for the most part, it's quite well designed and written.

    Now, there are other groups that use this framework. One group in particular, has pretty much the same standards that our group does. The difference is, however, that their manager never had them do code reviews and so people pretty much ignored the standards. I've now been tasked with working with that group and their code is a complete nightmare. For example, a single form class with something like 16 tab pages (spread among 3 or 4 tab controls), over 200 controls, and over 9000 lines of spaghetti code.

    Had this group done code reviews, this class never would have passed, and it wouldn't be such a nightmare to deal with. At this point, we're already shipping the second version, so a complete rewrite of the various nightmare components of this app are out of the question, which is too bad because it's going to be a nightmare to maintain, especially when the guy who wrote it leaves.

    I've always hated doing code reviews, but this experience has made it abundantly clear to me how important they are for minimizing the damage a single clueless programmer can get away with.

  • Style != substance (Score:3, Interesting)

    by melonman ( 608440 ) on Sunday July 20, 2008 @10:43AM (#24262191) Journal

    I've never been convinced by any hard-and-fast coding stylistics. Sure, it's possible to write good code and bad code, readable code and unreadable code, but beauty is very much in the eye of the beholder, and, also, it depends a lot what you are trying to do. Insisting on one inflexible set of stylistics works about as well as telling people never ever to split infinitives or never ever to use the word 'said'.

    Last night I came across this in the documentation for CPAN's Net::Server (you probably guessed from the above that I'm not a Pascal programmer):

    You may get and set properties in two ways. The suggested way is to access properties directly via
    my $val = $self->{server}->{key1};
    Accessing the properties directly will speed the server process - though some would deem this as bad style. A second way has been provided for object oriented types who believe in methods. The second way consists of the following methods:
    my $val = $self->get_property( 'key1' );
    my $self->set_property( key1 => 'val1');

    This struck me as remarkably sensible - the author of the module puts his prejudices on the table, but tells you how to do it a different way if you like. (And, personally, I prefer the first example, because it's just as clear, faster, and I've never managed to take OOP in perl entirely seriously - a problem that Larry Wall appears to have too.)

    You judge good style in any particular case by looking at the overall work, not by nit-picking about the punctuation in isolation.

  • by Fallen Andy ( 795676 ) on Sunday July 20, 2008 @10:50AM (#24262265)
    write; multiple; statements; per; line; for; 300; chars;

    If it's assembler then write pseudo ADA comments which bear no resemblance whatsoever to the badly commented code following - Bonus points if the pseudo code itself has bugs...

    If it's Delphi code make all units UNITx, all forms FORMx and all variables equally inanely named - if it's good enough for most Delphi books then obviously it's the right way to do things

    Avoid function prototypes - if it was good enough for Brian and Dennis it's good enough for anyone

    Overload operators in surprising and pleasing ways, preferably so that "-" does bitwise set inclusion

    Use macros extensively (without ()() because everyone knows only losers need them).

    Mix tabs and spaces indiscriminately

    Pick at least *two* styles for braces - Bonus points for gratuitously adding them where they aren't needed - (to really make the reader happy use the "{" on next line style here)(extra points if you are mixing tabs and spaces)

    Use if (1==x) , (x==1) and just plain old if (foo) randomly to add variety

    Write big huge case (switch) statements spanning 5 pages because no one would possibly understand dispatch tables

    Seriously though, if you're programming anywhere you're expected to conform to the local customs, wacky and out of it or not. It's part of the adaptability expected of a programmer...

    Andy

  • by MobyDisk ( 75490 ) on Sunday July 20, 2008 @10:51AM (#24262275) Homepage

    Forget this pointless stuff about tabs and spacing, I've seen some really brain-dead policies.

    1) Source Control Substitute
    At one shop, there were designers who edited XML + image files (kinda like web pages, but not quite). There was a compiler that built this all into a single executable. They were not permitted to edit the source directly, and had to work on copies. And those copies must be on the network instead of their local drive. And source control was not allowed.

    So instead of people having local copies and then committing their work, everyone made a duplicate copy on the network for each thing they did. It took hours to make the copies, and the compile times went from a few minutes to 45 minutes. Plus, the network drive kept running out of space due to all the gzillions of copies of everything.

    2) Making the "minimal" change required
    I worked for a US government contractor and they wanted each change to have the minimal impact on the system that was possible. So, basically nobody ever removed code, only added. One time I encountered a huge nested if statement that spanned hundreds of lines. Upon looking at the cases, I noticed that many of them were the same. Like:

    if (a)
        if (b)
            do x
        else
            do y
    else
        if (b)
            do x
        else
            do x

    which can, of course, be simplified to:
        if (a and not b) do y else do x

    This was because people had to make the MINIMAL change required each time a change was made. And removing a level of the if statements was more lines of code modified than just changing "do y" to "do x"

    Imagine this, but with dozens of cases spanning hundreds of lines. I spent almost a day to build a chart listing what each combination of variables did, and finally chopped hundreds of lines of code to about 10 lines. Turns out that after years of changes, most of the cases now did the same thing.

  • by yelvington ( 8169 ) on Sunday July 20, 2008 @11:01AM (#24262363) Homepage

    Some programmers think they should be able to do anything they want.

    That might be OK if you live in your parents' basement and code for yourself, but in the real world it's a bad (and selfish) idea.

    Strict adherence to a standard is helpful in code review and in cases where a component is taken over by a new maintainer.

    This is always important, but it's particularly important in a genuinely open, community-driven project.

    The Drupal project is an example. It has a coding standard derived from the PEAR project [drupal.org] that applies to any code submitted for inclusion in the core.

    Contrib authors are encouraged, but not required, to follow it. The good ones do.

    The Drupal Coder module [drupal.org] does a very good job of nagging at you until you get the formatting right, and also helps with code migration and updating when the API changes. And it finds some common bonehead mistakes that can create security issues.

    Adhering to a standard doesn't have to be painful. Using a properly configured text editor helps. There is good support for Drupal standards and conventions in OpenKomodo [openkomodo.com] and the commercial Komodo IDE, as well as some other editors.

  • by kevin_conaway ( 585204 ) on Sunday July 20, 2008 @11:14AM (#24262497) Homepage

    We found that it doesn't really help to enforce a *formatting* style on developers because everyone has their own. The only thing you really should be enforcing is tabs vs spaces (and it should be spaces) because mixing the two can produce some really ugly results.

    We have a much better rate of return running tools like JSLint or PMD to catch issues that are syntactically valid but will be sure to cause problems down the line

  • by RAMMS+EIN ( 578166 ) on Sunday July 20, 2008 @11:23AM (#24262575) Homepage Journal

    In a way, the languages, tools, and libraries prescribed (if any), also constitute a sort of coding practice, in the sense that they impose limits on how you can structure your code.

      - The language you work with gives you certain language constructs. These constructs vary per language, and determine how you must express things and what abstractions are available to you. This has a huge impact on the structure of your code.

      - Most tools like to structure and format your code a certain way, particularly when the tool generates the code. This is usually a great boon, because it will make it easy for programmers to adhere to the same coding standard and hard for them to deviate. Of course, if what you want is not what the tool wants, the tool starts getting in the way.

      - The libraries you work with determine the APIs available to you. This also has a strong influence on the structure of your code. It also interacts with the language constructs available to you, as they may or may not make it easy to build an API you like to work with on top of the API that a library exposes.

    Abstraction is particularly important. If a language offers powerful enough abstractions, you can structure your program so that it is easy for humans to understand what it does, and have the compiler translate it to whatever the libraries make available to you. Better abstractions also make your code more reusable.

    As an example, in C, strings are character arrays. Arrays in C don't have a size associated with them. The end of a string is indicated by a character with value 0. Furthermore, the type of an array of characters is actually the same as a pointer to a character. C also doesn't have automatic memory management. Suppose now that you wanted to concatenate two strings. There are various ways to do so, but the most obvious one is the strcat function:

    char *strcat(char *dest, const char *src)

    This function appends src to dest and returns dest (a pointer to the concatenated string).

    That is, provided there was actually enough space in dest to hold the combined contents of dest and src, and the terminating NUL. If there wasn't, the function overwrites whatever came after dest, which will usually lead to your program crashing or executing code supplied by a cracker attacking your program.

    The correct way to use strcat, then, is something like:

    /* The first string, the second string, and a to-be-allocated string for their concatenation. */
    char *first, *second, *result;
     
    /* Don't forget to add 1 for the terminating NUL character. */
    result = (char*) malloc(strlen(first) + strlen(second) + 1);
     
    /* Copy the contents of first to result. */
    strcpy(result, first);
     
    /* Append the contents of second. */
    strcat(result, second);

    But wait! That's not all! Since the type of an array is actually a pointer, and pointers are allowed to be NULL in C, first and second in the above could actually be NULL. If either one of them is, the program will crash. So we need to add extra code to check for that ...

    All those many things to remember to concatenate two strings. It doesn't have to be that way. In OCaml, for example, a string is a string, not a pointer to a character, and never null. You don't have to worry about allocating a large enough block of memory, because memory will be allocated as needed, and reclaimed when no longer reachable. As it happens, OCaml also has an operator that concatenates strings. That is besides the point here, but I had to tell you that to explain what the code looks like in OCaml. Namely:

    first ^ second

    Not only is it much shorter than the C code, it's also easier to understand what it does, and more robust.

    I think this sort of thing matters a lot more than how you format or indent your code, and pretty much everything else that normally falls under the nomer of "coding standards".

  • by dysfunct ( 940221 ) * on Sunday July 20, 2008 @11:37AM (#24262691)

    maybe continue or multiple return statements were off-limits.

    Now why would anybody do this? I've always assumed code like this was basically what inexperienced people would use:

    if(condition) {
    // 100s of lines of code
    return true;
    }
    else
    return false;

    Why not just return immediately if any basic conditions or assumptions are not met and prevent that completely unnecessary indentation? The only advantage I can see is that you could miss the return statement when reading the code.

  • by Animats ( 122034 ) on Sunday July 20, 2008 @12:23PM (#24263081) Homepage

    The prohibition on "multiple exits" or returns comes from a misunderstanding of early program proving technology. As one of the few people who ever built a real proof of correctness system [animats.com], I know that's just not a problem.

    There are some topological restrictions on program proving, but you can't violate them with "break" or "return". You need "goto" to really screw up. The actual topological constraint is that backwards control paths must not cross.

    The basic requirement for proving loops is that there must be some section in the loop through which control must pass on every iteration. Somewhere in that section must go the loop invariant and the termination measure.

    Nobody does this for software any more, although, interestingly, full-scale machine proof of correctness of hardware logic in VHDL is not that unusual. There are commercial tools for that.

  • by harlows_monkeys ( 106428 ) on Sunday July 20, 2008 @02:33PM (#24264415) Homepage
    When doing maintenance on someone else's code, use their style, even if it is one you hate.

A morsel of genuine history is a thing so rare as to be always valuable. -- Thomas Jefferson

Working...