User:Stuart/Scratchpad/Coding Standards

Stuart

edit this panel Address 42 Gloucester Avenue, Lancaster, LA1 4EU Phone 01524 849637 Mobile 07910 207218 Mine Diary Archived Notes Scratchpad Bugfix Sprint Coding Standards Scratchpad II IServer Usage Dev Support My Open Bugs All Open Bugs Rota Other Work Password Encryption Extending Groups Angel Work Queue Epic Active Work Queues Permissions Expansion Targeting .NET 4 Multiple Character Sets Office Procedures Character Matching Character Matching Schedule Calendars Internal Business Object Credential Provider User Access Control MAPIEx Resilience Useful Bits Category:Developer Help My Angel Bugs Other Users Ciaran Giles Glyn JohnB Alex Sam

Work In Progress

Coding Standards Take 2

Note to self Things I want to cover in this:

• VB standards (global)
• C# standards (global)
• General standards (ours)
• SQL naming standards
• SQL code conventions? .NET/SQL conventions?
• Suggestions from me.
• Git commit standards

Some todos:

• Clean-up
• Attributes
• Events (usage, more than naming)
  • As a corollary, the use of OnEventOccurred virtual methods
• SQL, Git
• Resources
  • Including how to use them, BluePrism.Images etc.
• UI Design - be as slaves to the visual designer for it is how everything works in VS.

Overview

It has become apparent that there is some disagreement over whether we follow any sort of coding standard at all. I submit that we do, but it is at a very high level, not a prescriptive set of 'commandments' that all code should follow - and I would like to keep it like that, with only functionally dangerous conventions being forbidden rather than stuff that one or two developers don't like the look of.

In general, coding standards exist so that it's one less barrier for a programmer skilled in the language to have to deal with when approaching a new project, so I think we should try and use the general standards and good practices that are largely agreed on within the larger programming community.

As such, I think the best place to start is the Microsoft coding conventions - they are quite liberal, and are detailed in the pages:

• Visual Basic Coding Conventions
• Visual Basic Naming Conventions
• C# Coding Conventions

I haven't really gone into it in detail, but it might also be worth looking at:

• Secure Coding Guidelines

We also have some coding standards, written mostly before my time (2009), which are some more restrictive and, possibly as a result, are largely not followed. These are on the wiki, currently at [Coding Standards]; it may be moved to another page as our current set of standards is promoted - if so, I'll update this reference.

I'm going to try and pull together a few different references here, namely:

• Microsoft's conventions
• Our conventions
• My own preferences

We can then argue about them at wholly unnecessary length and hopefully we'll have something that we can all agree with (or at least compromise on) at the end of it.

I was effectively copying all standards that I found into this page, but that ended up seeming pointless (plus there was quite a bit of stuff which was just not relevant to us), so I'm going to list some standards

General

File and path names

• Generally, folder and file names should not contain characters other alphanumeric ([A-Za-z0-9]), hyphen, dot.
  • An exception to this might be user-orientated documentation - eg. a user guide maintained by us - which might contain space characters

Note: I'd like to go through our code at the end of a release and remove all the spaces from our folder names. They're really painful.

Tabs vs Spaces

There's no 'correct' answer - people have preferences, but consistency is important. Microsoft's Visual Basic Coding Conventions suggests:

Insert tabs as spaces, and use smart indenting with four-space indents.

Spaces seem to be the more widely accepted choice in style guides I've found online, but it seems that most of our code leans towards tabs. Whichever we choose, we should apply it accorss the board for all languages and all dev output (eg. HTML, XML, JSON, C#, VB, C++, Python, CSS, Javascript, anything else).

Incidentally, I quite like the Elastic Tabstops idea, but that has somewhat limited implementation.

Code organisation

Where do I start?

Our code organisation is all over the place - especially the Automate project. We should try and agree how it should be organised and then figure out how we can get it there. Some ideas:

• Directories should follow namespaces - eg. BluePrism.Core.Expressions should be in the Expressions/ directory inside the root BluePrism.Core directory.
• Some of our directories are in subdirectories beneath the automate root, some are directly in that root; each project should have its folder directly in the automate root - ie. not in Automate-3.0, ApplicationManager.
  • ...or... should we have a subfolder inside automate for the product code ("Product"? "Main"?) - separating each project from each other is unhelpful, but separating our core product from Login Agent, BPJabInstaller, Release, etc. seems sensible.
  • ...or... should we go all the way in our 'directories follow namespaces' rule and have a structure beginning with our root namespace BluePrism, and have the sub-folders map into namespaces from there, so the above example would have:
• BluePrism/
  • Core/
    • Expressions/
    • Data/
    • Filters/

Unit Tests

Current standard-ish is to have a _UnitTests folder (note the leading underscore, which, itself, breaks several styling guidelines) inside which the unit tests are defined in no specific namespace (typically the 'main' namespace of the class(es) that the fixture is testing).

I propose a UnitTests folder which follows the namespace rules above - eg. in Core, a folder: $/BluePrism/Core/UnitTests with the tests defined in a namespace BluePrism.Core.UnitTests.

Events

The .NET convention is for each defined event to take two parameters:

• Object sender

  The source of the event - the object on which the event was fired.

• XXXEventArgs e

  An EventArgs or subclass instance. All extra data provided by the event should be declared in the specific instance of the args provided.

Some standard EventArgs subclasses include:

CancelEventArgs

Used to provide a mechanism to cancel an event before it completes

MouseEventArgs

Provides data on mouse locations and button presses

... there are many others.

Even if no extra information is required within the event, the (Object, EventArgs) signature should be used - an empty event args object can be retrieved using EventArgs.Empty in that circumstance.

Another convention is for each class in which an event can be raised to defined a protected virtual OnEventOccurred method which just takes the EventArgs or subclass argument and uses that to raise the event. Not everything does it, but it always feels jarring when something doesn't, so I think we should.

VB doesn't require the creation of EventHandler delegates for events, but C# does and it's a convention used throughout the .NET libraries, so I think we should too.

The way I tend to do it Template:FIXME

.Net Code

There's a general set of guidelines at Microsoft's Framework Design Guidelines. I did start to transcribe them here, but it might be better if we just go through them and collect the ones that we want to adopt.

Commenting

• Non-private members should be XML-documented
• XML-documentation is not necessary for private member variables
• Though I think properties, methods, inner classes et al should still be XML-documented.

Naming

Micrsoft has a huge naming section in its design guidelines - see the Naming Guidelines. I'll be referring to this from time to time below.

Note: I've used Microsoft's convention of 'camel case', starting with a lowercase letter and 'Pascal case' starting with an uppercase letter, eg.

• PascalCase
• camelCase

General

The Microsoft General Naming Conventions has some sensible stuff to say about this:

• DO choose easily readable identifier names.

For example, a property named HorizontalAlignment is more English-readable than AlignmentHorizontal.

• DO favor readability over brevity.

The property name CanScrollHorizontally is better than ScrollableX (an obscure reference to the X-axis).

• DO NOT use underscores, hyphens, or any other nonalphanumeric characters.
• DO NOT use Hungarian notation (lpszMyString, sErr, bFound etc.)
  • You could argue that this precludes using a prefix for private member variables. Discuss.
  • Also Joel Spolsky makes a good case for true Hungarian notation - I think that would take a lot more thinking about and it would end up getting confused with the 'type-based' Hungarian notation we have scattered around the code.
• AVOID using identifiers that conflict with keywords of widely used programming languages.
• DO NOT use abbreviations or contractions as part of identifier names.

For example, use GetWindow rather than GetWin.

• DO NOT use any acronyms that are not widely accepted, and even if they are, only when necessary.
• DO use semantically interesting names rather than language-specific keywords for type names.

For example, GetLength is a better name than GetInt.

• DO use a generic CLR type name, rather than a language-specific name, in the rare cases when an identifier has no semantic meaning beyond its type.

eg. use GetInt64 rather than GetLong; GetSingle rather than GetFloat;

Namespaces

• Pascal Case
• Should start with "BluePrism" (from the naming guidelines: DO prefix namespace names with a company name to prevent namespaces from different companies from having the same name)
• Should not be the same name as a type inside that namespace
• Filenames should follow the directory structure from the root namespace - eg. BluePrism.Core.Expressions should be in the Expressions/ directory inside the BluePrism.Core directory.
  • Not unlike the 'removing spaces' above, I would like to re-organise our code, especially the Automate project, such that it follows this standard. But that's not a small task and it needs separate planning / discussion.
• The general aim (from the naming guidelines) is:

<Company>.(<Product>|<Technology>)[.<Feature>][.<Subnamespace>]

Namespace level elements (class, struct, interface, enum, module)

• Pascal Case
• No 'type' prefixes (a personal bugbear of mine with our code) except for interfaces (Nnnngh! See below)
• If appropriate, suffix with a type - eg. UserDetailsForm, ImageButton, NameChangedEventArgs

Interfaces

• Begin with a capital I. Ick. Holdover from COM, but that is the .NET standard, so we do the least surprising thing and follow it.

Type Members (methods, properties, inner classes, events)

• Pascal Case

Methods

• Typically should be verbs or verb phrases (eg. CompareTo, Split, GetHashCode
• Shouldn't have properties which match the name of 'Get' or 'Set' methods in the type, eg.
  • public string TextWriter { get; }
  • public string GetTextWriter() {...}

Properties

I'm leaning on MS's Names of Type Members guidelines a little here:

• DO Name properties using a noun (eg. Name, Description), noun phrase (eg. {{c|

Member variables:

• Start with an underscore followed by camel case - _xxXxxXxx
• Start with "m_" followed by camel case - m_xxxXxxx
• Start with "m" followed by Pascal case - mXxxxXxxx
• Simple camel case name - xxxXxx - distinguish when ambiguous using a this.xxxXxx or Me.xxxXxxx
• Different for VB / C#?
• No convention, just do what you want.

Discuss.

Classes, Structures, Enums

Starting with prefix indicating broad 'type' of element:

• Control - ctlXxxXxx
• Form - frmXxxXxx
• Module - modXxxXxx
• "Class" - clsXxxXxx

Yeah, this is a relic of VB6 which I would like to burn - I've never liked it. "Control" and "Form" are classes, what about Enums? Structures? I prefer, from the Microsoft naming guide:

• DO choose easily readable identifier names.

For example, a property named HorizontalAlignment is more English-readable than AlignmentHorizontal.

• DO favor readability over brevity.

The property name CanScrollHorizontally is better than ScrollableX (an obscure reference to the X-axis).

So, for me - suffix with the broad type of element where appropriate, so that it's easily readable as English(ish) eg:

• HelpButtonForm
• HighlighterWindow
• RowBasedDataGridView
• EventFiringDictionary
• BackgroundThreadScheduler
• OperationCompletedEventArgs
• NoSuchQueueException

---

Style

• Code to 86 characters. Which seems arbitrary because, as far as I can tell, it is.

Block size

I found this in some random site somewhere and it was talking about C code, but:

• Don't put closing braces more than one screen away from the matching opening brace

... seems like a really good idea which we certainly don't apply at the moment... should we?

Regions

Regions are a neat way of structuring a large source file, so that it's clear which section of the code that something is in. We have multiple 'region strategies' employed in our code

No regions at all

For small classes (<4 pages), regions may just add noise without appreciable gain.

Region all the things

Every element has a region - each method, then a group of methods into some grouping. Thankfully not used too much.

Add regions by functional area

"Security Methods", "Xml Handling", "Display Functionality", etc. This works, up to a point, but then you have something which fits in 2 or more of the arbitrary groupings and you have to guess which one the original developer settled on, or whether they weren't sure so they dropped it in the inevitable "General" region - and, suddenly, regions inhibit readability rather than enhance it.

Add regions by type

My preference - group together "things of the same type", ie.

• "Class-scope Declarations" - ie. inner classes, enums, constants, static declarations
• "Member Variables" - instance members
• "Auto-properties" - Automatic Properties - ie. properties with implicit get/set definitions
• "Constructors" - um....
• "Properties" - Any properties with explicit get/set definitions
• "Methods"

I sometimes split the methods into "Event Handling Methods" and "Other Methods"

But there's no ambiguity about where something should go - an element fits into exactly one region, and it means that all code within a class ends up in a region, for neatness.

On a not-unrelated note, TODO: ... and that's where I left this sentence weeks ago. I must try and figure out what not-unrelated subject I had in mind

---

Design Principles

Exceptions

• Create reasonably specific exceptions for specific purposes, or use one of the medium-level exceptions available in the core libraries, eg. NoSuchElementException or PermissionException.
• Specifically, don't use the vague ApplicationException; from the MSDN ApplicationException page:

If you are designing an application that needs to create its own exceptions, you are advised to derive custom exceptions from the Exception class. It was originally thought that custom exceptions should derive from the ApplicationException class; however in practice this has not been found to add significant value. For more information, see Best Practices for Handling Exceptions.

Properties

Although properties are technically very similar to methods, they are quite different in terms of their usage scenarios. They should be seen as smart fields. They have the calling syntax of fields, and the flexibility of methods.

✓ DO create get-only properties if the caller should not be able to change the value of the property.

Keep in mind that if the type of the property is a mutable reference type, the property value can be changed even if the property is get-only.

X DO NOT provide set-only properties or properties with the setter having broader accessibility than the getter.

For example, do not use properties with a public setter and a protected getter.

If the property getter cannot be provided, implement the functionality as a method instead. Consider starting the method name with Set and follow with what you would have named the property. For example, AppDomain has a method called SetCachePath instead of having a set-only property called CachePath.

✓ DO provide sensible default values for all properties, ensuring that the defaults do not result in a security hole or terribly inefficient code.

✓ DO allow properties to be set in any order even if this results in a temporary invalid state of the object.

It is common for two or more properties to be interrelated to a point where some values of one property might be invalid given the values of other properties on the same object. In such cases, exceptions resulting from the invalid state should be postponed until the interrelated properties are actually used together by the object.

✓ DO preserve the previous value if a property setter throws an exception.

X AVOID throwing exceptions from property getters.

Property getters should be simple operations and should not have any preconditions. If a getter can throw an exception, it should probably be redesigned to be a method. Notice that this rule does not apply to indexers, where we do expect exceptions as a result of validating the arguments.

Python

Code should conform to PEP8, apparently. (Taken from Coding Standards#Python.)