My Coding Conventions

When I run my own company or lead a brand new team, everyone will follow coding conventions. My Coding Conventions!

Here they are, to the best of my current knowledge and by probable order of importance.

  1. Every method will be documented! That includes purpose, parameters, proper usage, etc…
  2. Every property will be documented! Yes. Exactly the same.
  3. Every class will be documented! Especially about recommended uses and where it is suppose to go.
  4. Every public field which is not incredibly obvious will also be documented!
  5. Every conditional block will be an actual block, with parentheses. Even if it’s one line!
  6. Starting parentheses always get a new line! Exception: if it’s a conditional or property and it fits in one line.
  7. Every method shall appear in full before its first use. This makes the code more readable! Do It!
  8. Design for any part of the program should be ironed out ahead of time and with as much detail as reasonableness allows.
  9. To be continued… (Suggestion welcome)

Posted in IT, Practice, Programming, Thinking Out Loud by with 6 comments.

Comments

  • עודד says:

    1. What do you think about self documented code? Read: http://blog.codinghorror.com/coding-without-comments . Generally I prefer well written self-documented code than well documented code, but just forcing people to write comments doesn’t actually encourage good readability. I suggest replacing that with “Code reviewers are encouraged to reject changes based on readability alone”.
    2. See above, but I think this is a worse problem: while adding 2-3 lines of comments to a large method doesn’t decrease SNR significantly, especially if the comments are good, adding 2-3 lines of comments to a single property definition massively reduces readability, especially if the property name is actually well self-documenting (as it should be by default).
    3. Ok, I’ll grant that – class documentation is generally a good idea.
    4. see 2.
    5. I would tend to say “lets agree to disagree”, but seriously, here is a line I wrote yesterday (the language is Ruby):

    next unless value.valid?

    and here’s how you would have me write it according to your coding convention:

    if !value.valid? then
    next
    end

    Are you really telling me that for readability and ease of maintenance you would have preferred the second version?
    6. Well, if it doesn’t fit in a single line, then we should break it up, right?
    7. Not always actually doable – sometimes methods call each other in an order to make this impossible. In the past (i.e. Pascal) we forced developers who want to call methods before they are implemented to do a “forward declaration”, then we decided its not that important and stopped doing that.

    Well, half your coding conventions are “document everything”, which I think is better addressed as a single rule of “if your code reviewer doesn’t agree with you that the code is well self-documented, write documentation”.

    As for suggestions: add a code review process! In my experience, code reviewing is the fastest and cheapest way to increase code quality. Something like “every commit to the main branch must be signed off by a developer of at least the same level”. This kind of process is very hard in the company I work for as we generally don’t have more than two people working on the same technology (one of the problems of highly opinionated and very experienced developers with a service oriented architecture), but I’m working on that :-)

    • Eran says:

      1) I’m all for self documenting code. I try to make my code read like plain English as much as I can. What I mean by this is to add a short comment on top of the method so that, if I’m reading a method that uses it and want to get a better understanding of what it does beyond its name, I could hover my cursor over it and get a bit more information.
      2) By this, I mean a one line comment about what does this property do beyond receive and retrieve data to get the same effect as 1.
      4) Some times this is necessary too, especially when you give a new module to someone who needs to use it and you want them to “Get it” as quickly as possible.
      5) Here is a piece of my code:
      if (something == null) { continue; }
      Here’s another piece:
      if (something == null) { sendWarning(); }
      else if (!IsValid(parameter)) { doSomething(); }
      else { something = parameter; }
      The point in this is to always include the blocks. If you don’t and then later decide to add something then your code might get mangled when you add something after it but it’s not really part of the block.
      6) This is to prevent people writing:
      method() {
      lots of code
      }
      In my opinion, that makes the code much harder to read than:
      method()
      {
      lots of code
      }
      It’s simple and (some people might consider) silly but it makes a huge difference to me.
      7) I understand that. But again, I try to make my code speak for itself and wherever possible, if I can make my code more legible by explaining things before I use them and not making people search for them, I think that’s better. This is, of course, addressed at people who get a new class they don’t know and try to read it from start to finish.

      I tried “Document things that are not understandable in a second” but that’s to general and sometimes specifics are required. I like to be specific, it prevents misunderstandings.

      Code review could be great but it’s problematic because it required a lot more time. I would prefer to iron out design ahead of time and trust the people I work with are professionals.

      • עודד says:

        5. I think these braces are redundant line noise.
        6. I disagree. I hate too much empty space – it decreases readability. Like you don’t take an empty line (or mostly empty – has only an opening brace) after an “if”, you don’t take one after a function decleration. When readability is at stake, concise is better.

      • עודד says:

        [starting a new branch for easier tracking]

        The “I trust people to be professionals” argument can be used to knock down all of your rules (especially the one about always braching blocks – the common reason to want to do that is to prevent the next developer adding more lines to an unbraced blocks without adding the braces, a clearly unprofessional move).

        Code review doesn’t take more than a few minutes, unless the change is very complex at which point you really do want to have another developer spend some time on it. The way I see it, any code that goes into production should pass through at least two developers (with very few exceptions). One way to achieve this is pair programming, which if you can do that then that is awesome, but otherwise – use code review. There are even specialized tools that makes this very easy.

  • Eran says:

    6. Get Good at Writing Documentation

    Yeah, yeah, we all know how important it is to document our code, right? Part of the reason why programmers tend to have a love-hate relationship with writing documentation is because we’re typically not fans of being forced to do things that we think are unnecessary. But this sentiment mostly comes from a confused understanding of what documentation is for. I tend to see documentation as mainly being a tool for communication, meaning that when I write comments, my task is to make sure that whoever is reading it (whether it be a team member, a future employee, or myself) is able to understand the code that it describes. When people resist writing documentation, they usually say it’s unnecessary because either: (1) the code is obvious, (2) everyone on the team already understands it, or (3) there’s no way that they’re going to forget what the code does.

    Because it’s so easy to make bad assumptions (such as the ones stated above), I personally prefer to take a risk-oriented approach towards writing documentation. For instance, instead of asking myself “Does everyone already understand this?”, I instead ask “What are the chances that someone will ever be confused by this?” If I think there’s even a small chance, then that’s usually good enough reason to write at least something down.

    It also helps to show your code to someone and to actually see how they’re confused by it, so that you have a clearer idea of what points of confusion your documentation is supposed to help fix. Much in the same way that bugs are fixed after discovering them, many good comments are written after finding those points of confusion.

    http://gamasutra.com/blogs/LivioDeLaCruz/20140702/220177/Unconventional_Tips_for_Improving_your_Programming_Skills.php

    • עודד says:

      Documentation is important, I don’t think a discussion of *whether its impotant* has any merit in civilized society. But an important conversation is *how to document* – because I often see a lot of documentation that is perfectly useless because it either just repeats what is clearly in the code or repeats what should have been in the code if the code was using proper names for methods and variables.

      I’m a huge proponent of “the code is the documentation”, for documenting what the code *does*, but code comments are incredibly important for *why* the code does what it does. Examples:

      * Bad:

      twiddle_appropriately(); // send the twiddle command to the knob controller

      * OK:

      twiddle_knob();

      * Good:

      twiddle_knob(); // if we don't twiddle the knob at least once in 5 minutes, the next command may fail

      And now a real example from a project in work:

      class Environment
        def create(opts = {})
          servers = []
          servers << build_api_server(opts)
          if !(opts[:dev] or opts[:staging]) # need to start a database as well
            servers << InstanceManager.start_instance(db_cloudinit)
          end
            
          if opts[:workers]
            leading_instance.add_tag('RequiredWorkers', value: opts[:workers])
            servers += build_workers
          end
          unless opts[:use_wowza]
              servers << build_publish_server
          end
          setup_storage
          setup_elb
          Orchestration::Alarms.new.setup(self)
          # let CMS know about the new environment
          @owner.s3.buckets['cms.showbox.com'].objects.create("envs/#{name}", "")
          deploy_applications
          unless opts[:copy].nil?
            copy_data_from(opts[:copy])
          end
        end
         ... other methods not shown ...
      end
      

      This is what I consider well documented code. Yes, it has only two comments.