View source for User:Stuart/Scratchpad/Coding Standards

{{nav/stuart}}
{{stub}}
= 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 {{c|OnEventOccurred}} virtual methods
* SQL, Git
* Resources
** Including how to use them, {{c|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:
* [https://msdn.microsoft.com/en-us/library/h63fsef3.aspx Visual Basic Coding Conventions]
* [https://msdn.microsoft.com/en-us/library/0b283bse.aspx Visual Basic Naming Conventions]
* [https://msdn.microsoft.com/en-gb/library/ff926074.aspx C# Coding Conventions]

I haven't really gone into it in detail, but it might also be worth looking at:
* [https://msdn.microsoft.com/en-us/library/8a3x2b7f.aspx 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

== Points of discussion ==
How to organise this document? I was really struggling to make it coherent (should comments have their own section? should comment standards go into each section dedicated to the elements that they are commenting? Likewise with naming).

Should we introduce something to police this? Something on the build server? On independent dev boxes, eg. ReSharper or such like?

QA Tests? Should we have a standard for them - I'm talking about the VBOs / processes more than anything else (Python is... well... "touched on" below).

== 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 [https://msdn.microsoft.com/en-us/library/h63fsef3.aspx Visual Basic Coding Conventions] suggests:
{{q|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 [http://nickgravgaard.com/elastic-tabstops/ 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. {{c|BluePrism.Core.Expressions}} should be in the {{c|Expressions/}} directory inside the root {{c|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 {{c|BluePrism}}, and have the sub-folders map into namespaces from there, so the above example would have:
* {{c|BluePrism/}}
** {{c|Core/}}
*** {{c|Expressions/}}
*** {{c|Data/}}
*** {{c|Filters/}}

==== Unit Tests ====
Current standard-ish is to have a {{c|_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 {{c|UnitTests}} folder which follows the namespace rules above - eg. in Core, a folder: {{c|$/BluePrism/Core/UnitTests}} with the tests defined in a namespace {{c|BluePrism.Core.UnitTests}}.

==== Events ====
The .NET convention is for each defined event to take two parameters:
* {{c|Object}} sender
*: The source of the event - the object on which the event was fired.
* {{c|XXXEventArgs}} e
*: An {{c|EventArgs}} or subclass instance. All extra data provided by the event should be declared in the specific instance of the args provided.
Some standard {{c|EventArgs}} subclasses include:
; {{c|CancelEventArgs}}
: Used to provide a mechanism to cancel an event before it completes
; {{c|MouseEventArgs}}
: Provides data on mouse locations and button presses
... there are many others.

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

Another convention is for each class in which an event can be raised to defined a protected virtual {{c|OnEventOccurred}} method which just takes the {{c|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 {{c|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 is to create a single file named after the EventArgs class, with the first (code) line of the file being the delegate which is used to handle events of that type, eg. see {{src|BPCoreLib/Events/NameChangedEventArgs.vb}}:
 '''Public Delegate Sub''' NameChangedEventHandler( _
  '''ByVal''' sender '''As Object''', '''ByVal''' e '''As''' NameChangedEventArgs)
 
 '''Public Class''' NameChangedEventArgs : '''Inherits''' BaseNameChangeEventArgs
    '' ' Rest of class snipped ''
 '''End Class'''

== .Net Code ==
There's a general set of guidelines at [https://msdn.microsoft.com/en-us/library/ms229042.aspx Microsoft's Framework Design Guidelines], which I used quite heavily to come up with a lot of the following - where you see '''DO''' or '''DO NOT''' or '''AVOID''' - that's usually from somewhere in those guidelines and, as a set of basic guidelines for readable, ''unsurprising'' code, they're pretty good.

=== 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.
* In the code, "why, not how" - ie. why is this occurring, not how is it occurring - the latter is redundant because the code already explains how.
** As for how much - that has to be played by ear; I tend to put in a lot of comments for any non-trivial work, because that's how I clarify to myself what needs to occur in a given block of code.

=== Naming ===
Micrsoft has a huge naming section in its design guidelines - see the [https://msdn.microsoft.com/en-us/library/ms229002.aspx 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 [https://msdn.microsoft.com/en-us/library/ms229045.aspx Microsoft General Naming Conventions] has some sensible stuff to say about this: {{q|
* '''DO''' choose easily readable identifier names.
For example, a property named {{c|HorizontalAlignment}} is more English-readable than {{c|AlignmentHorizontal}}.
* '''DO''' favor readability over brevity.
The property name {{c|CanScrollHorizontally}} is better than {{c|ScrollableX}} (an obscure reference to the X-axis).
* '''DO NOT''' use underscores, hyphens, or any other nonalphanumeric characters.
* '''DO NOT''' use Hungarian notation ({{c|lpszMyString}}, {{c|sErr}}, {{c|bFound}} etc.)
** You could argue that this precludes using a prefix for private member variables. Discuss.
** Also Joel Spolsky makes [http://www.joelonsoftware.com/articles/Wrong.html 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 {{c|GetWindow}} rather than {{c|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, {{c|GetLength}} is a better name than {{c|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 {{c|GetInt64}} rather than {{c|GetLong}}; {{c|GetSingle}} rather than {{c|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. {{c|BluePrism.Core.Expressions}} should be in the {{c|Expressions/}} directory inside the {{c|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) ====
See [https://msdn.microsoft.com/en-us/library/ms229040.aspx Names of Classes, Structs and Interfaces], which I borrow from quite heavily in the following sections.
* Pascal Case
* '''DO''' name classes and structs with nouns or noun phrases, using PascalCasing.
* '''DO''' name interfaces with adjective phrases, or occasionally with nouns or noun phrases.
* Begin interface names with a capital I. Ick. Holdover from COM, but that is the .NET standard, so we do the least surprising thing and follow it.
* '''DO NOT''' give class names a prefix (e.g., "C" or "cls").
* No 'type' prefixes (a personal bugbear of mine with our code) except for interfaces (Nnnngh! See below)
* If appropriate, ''suffix'' with a type - ''eg:''
** {{c|HelpButtonForm}}
** {{c|HighlighterWindow}}
** {{c|RowBasedDataGridView}}
** {{c|EventFiringDictionary}}
** {{c|DaySetCalendar}}
** {{c|OperationCompletedEventArgs}}
** {{c|NoSuchQueueException}}
*: This has to be a judgement call on the part of the programmer... or should we have some detailed? MS details a few [[#Common Types|which I go over below]].

==== Common Types ====
When deriving from or implementing various .NET framework types, use an appropriate suffix:

; {{c|System.Attribute}}
:
:* Add the suffix {{c|Attribute}} to names of custom attribute classes
; {{c|System.Delegate}}
:
:* Add the suffix {{c|EventHandler}} to names of delegates that are used in events.
:* Add the suffix {{c|Callback}} to names of delegates ''other than'' those used as event handlers.
; {{c|System.EventArgs}}
:
:* Add the suffix {{c|EventArgs}}
; {{c|System.Exception}}
:
:* Add the suffix {{c|Exception}}
; {{c|IDictionary}}, {{c|IDictionary&lt;TKey,TValue&gt;}}
:
:* Add the suffix {{c|Dictionary}} (I would prefer {{c|Map}} myself - a holdover from my java days which is ''far'' easier to type, but the standard and the current .NET types all use dictionary, so... grudgingly...)
; {{c|IList}}, {{c|IList&lt;T&gt;}}
:
:* Add the suffix {{c|List}}
; {{c|ISet&lt;T&gt;}}, {{c|IBPSet&lt;T&gt;}}
:
:* Add the suffix {{c|Set}}
; {{c|ICollection}}, {{c|ICollection&lt;T&gt;}}
:
:* Add the suffix {{c|Collection}}

==== Generic Type Parameters ====
* Pascal Case
* If a type is self explanatory (''eg.'' the type of element in a collection), consider using just {{c|T}}.
* For more descriptive type parameter names, begin with an uppercase {{c|T}} (''eg.'' {{c|TKey}}, {{c|TValue}}, {{c|TSource}}, {{c|TDest}})
* Consider indicating constraints placed on a type parameter in the name of the parameter (''eg.'' {{c|TEnum}})

==== Static Fields ====
MS only applies these guidelines to static public and protected fields. Internal and private fields are not covered by guidelines, and public or protected instance fields are not allowed by their [https://msdn.microsoft.com/en-us/library/ms229059.aspx member design guidelines].
* '''DO''' use PascalCasing in field names
* '''DO''' name fields using a noun, noun phrase or adjective
* '''DO NOT''' use a prefix for field names
*: For example, do not use {{c|g_}} or {{c|s_}} to indicate static fields.

In fact, I think the above should apply to internal and private static fields too - I see no reason to treat them any differently.

==== Type Members (methods, properties, inner classes, events) ====
* Pascal Case

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

==== Properties ====
I'm leaning on MS's [https://msdn.microsoft.com/en-us/library/ms229012.aspx Names of Type Members] guidelines a little here:
* '''DO''' Name properties using a noun (eg. {{c|Name}}, {{c|Description}}), noun phrase (eg. {{c|MaximumSize}}, {{c|PreferredWidth}}) or an adjective (eg. {{c|Enabled}}, {{c|DroppedDown}}), optionally prefixed with {{c|Is}}, {{c|Can}} or {{c|Has}} for boolean properties, "but only where it adds value" (that's from the MS guidelines, I'm not sure it adds value in itself).

====  Member variables: ====
* Start with an underscore followed by camel case - {{c|_xxXxxXxx}}
* Start with "m_" followed by camel case - {{c|m_xxxXxxx}}
* Start with "m" followed by Pascal case - {{c|mXxxxXxxx}}
* Simple camel case name - {{c|xxxXxx}} - distinguish when ambiguous using a {{c|this.xxxXxx}} or {{c|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 - {{c|ctlXxxXxx}}
* Form - {{c|frmXxxXxx}}
* Module - {{c|modXxxXxx}}
* "Class" - {{c|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 [https://msdn.microsoft.com/en-us/library/ms229045.aspx Microsoft naming guide]:
{{quote|
* DO choose easily readable identifier names.
For example, a property named {{c|HorizontalAlignment}} is more English-readable than {{c|AlignmentHorizontal}}.
* DO favor readability over brevity.
The property name {{c|CanScrollHorizontally}} is better than {{c|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:
* {{c|HelpButtonForm}}
* {{c|HighlighterWindow}}
* {{c|RowBasedDataGridView}}
* {{c|EventFiringDictionary}}
* {{c|BackgroundThreadScheduler}}
* {{c|OperationCompletedEventArgs}}
* {{c|NoSuchQueueException}}

==== Enums ====
* '''DO''' use a singular type name for an enumeration unless its values are bit fields.
* '''DO''' use a plural type name for an enumeration with bit fields as values, also called flags enum.
* '''DO NOT''' use an "Enum" suffix in enum type names.
* '''DO NOT''' use "Flag" or "Flags" suffixes in enum type names.
* '''DO NOT''' use a prefix on enumeration value names (e.g., "ad" for ADO enums, "rtf" for rich text enums, etc.).

=== Style ===
* Limit code width to 86 characters where possible. Which seems arbitrary because, as far as I can tell, it is.
** You can add guides to Visual Studio - see [Visual Studio Column Guides] for details (though I half-remember that this might not work in VS2013... I use something different now, so I can't remember for certain).

==== Imports ====
* Use imports rather than fully (or partially) qualified names for types in a file
* Only use aliases in imports to refer to inner types with brevity (although if you're referring to an inner type elsewhere, it's a big flag that it probably shouldn't be an inner type).

==== 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 ===
* Learn the constructs of the language which are designed to make the code easier to read/write or more concise and don't be afraid of using them, eg.
** Use literal XML where appropriate in VB.NET
** Use the null coalescing operator in C# ({{c|str ?? ""}}) or VB ({{c|If(str, "")}})
** Single-line If statements ({{c|If x Is Nothing Then Return Nothing}}) are fine.
** ... even if they include Else statements ({{c|If x = "" Then Return "" Else Return x}} - which may look entirely redundant, but that brings me to my next point)
** ("=" in VB) <> ("{{=}}{{=}}" in C#)
*** eg. You can do {{c|If myGuid {{=}} Nothing Then}} in VB to check if a Guid is empty, or, much more importantly...
*** You can do {{c|If myString {{=}} "" Then}} in VB to check if a string ''is null or empty'' - ie. a null string is treated as equal to an empty string for the purposes of the equals operator. If the distinction is important, use {{c|Is}} (eg. {{c|If myString Is Nothing}}) or {{c|Equals}} (eg. {{c|If String.Equals(myString, "")}}).
** {{c|Handles}} is the single best thing about VB.NET in my humble... use it over {{c|AddHandler}} and {{c|RemoveHandler}} where possible.

==== Structures vs Classes ====
* '''DO''' use classes
* Why are you still here? Use classes!

==== Inner Types ====
See [https://msdn.microsoft.com/en-us/library/ms229027.aspx MS Type Design Guidelines -> Nested Types].

* '''DO''' use nested types when the relationship between the nested type and its outer type is such that member-accessibility semantics are desirable.
** Basically, nested types have scope to access to everything in the outer class that code in the outer class has access to - when this is important/useful, ''that'' is when it's useful to use a nested type - the archetypal example is an {{c|IEnumerator}} for a collection. It's useful for the enumerator to have direct access to the underlying data and is unlikely to be called in any other way than through its owning collection class.
* '''DO NOT''' use nested types for anything else, really. But specifically:
** '''DO NOT''' use public nested types as a logical grouping construct - use namespaces for this.
** '''AVOID''' publicly exposing nested types.
** '''DO NOT''' use nested types if the type is likely to be referenced outside of the containing type.
**: For example, an enum passed to a method defined on a class should not be defined as a nested type in the class.
** '''DO NOT''' use nested types if they need to be instantiated by client code. If a type has a public constructor, it should probably not be nested.

==== Exceptions ====
* Create reasonably specific exceptions for specific purposes, or use one of the medium-level exceptions available in the core libraries, eg. {{c|NoSuchElementException}} or {{c|PermissionException}}.
* Specifically, don't use the vague {{c|ApplicationException}}; from the [https://msdn.microsoft.com/en-us/library/system.applicationexception%28v=vs.100%29.aspx MSDN ApplicationException page]:
{{q|
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 [https://msdn.microsoft.com/en-us/library/seyhszts(v{{=}}vs.100).aspx 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.
* '''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.
*:* We have a {{c|WriteOnly}} property somewhere; can't remember where, but we should zap it when we find it.
* '''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.
* '''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.

=== User Interface Design ===
* Is this appropriate for a standards document?
* As much as possible, our controls should attempt to fit into the Visual Studio standard - ie. to be usable from the visual designer. This may mean compromises on language features, such as:
** Inheritance - the Visual Designer has some serious problems with inheritance hierarchies.
** Exposing arrays and collections without designer classes - programmatical configuration is not possible with the designer unless designer classes are written to a) design them and b) serialize them to a {{c|.Designer}} file.
** Using properties as much as possible to configure the controls
** Limiting constructors to a single public no-arg constructor

== SQL ==
* Use camel case for tables & views
* Use all lowercase for columns and variables
* Prefix tables with {{c|BPA}} eg. {{c|BPAWorkQueueItem}}, {{c|BPAUser}}
* Prefix views with {{c|BPV}} eg. {{c|BPVSessionInfo}}, {{c|BPVWorkQueueItem}}
* Prefix custom stored procedures with {{c|usp_}} - this is a standard for user stored procedures to distinguish them from preinstalled stored procedures which are prefixed {{c|sp_}} - eg. {{c|sp_help}}, {{c|sp_start_job}}. See [https://msdn.microsoft.com/en-us/library/dd193246(v=vs.100).aspx Transact-SQL Naming Issues], specifically [https://msdn.microsoft.com/en-us/library/dd172115(v=vs.100).aspx SR0016].
* When writing a long SQL query in .NET code, write it over a number of lines using literal string concatenation and use indentation to make it easy to follow. I tend to start each line with a space char so I know that I'm not accidentally leaving a space out and joining two words together, eg.
 '''Dim''' cmd '''As New''' SqlCommand(
  " select" &
  "   r.id as releaseid," &
  "   r.name as releasename," &
  "   r.created," &
  "   r.local," &
  "   u.username," &
  "   r.notes, " &
  "   re.typekey," &
  "   re.entityid as id," &
  "   re.name" &
  " from BPARelease r" &
  "   join BPAUser u on r.userid = u.userid" &
  "   join BPAReleaseEntry re on re.releaseid = r.id" &
  " where packageid=@packageid"
 )
* Also, when writing .NET code to access the database, ensure that you match the case of the database, even if it deviates from these standards.
** (Or use a {{src|BPCoreLib/Data/ReaderDataProvider.vb}} to read the data - that ensures that an invariant culture is used to see if a column is returned in a {{c|SqlDataReader}}.)

== Python ==
* Code should conform to [http://www.python.org/dev/peps/pep-0008/ PEP8], apparently. (Taken from [[Coding Standards#Python]].)
** Should we maybe elucidate on this somewhat?
* Spaces/tabs should be consistent
* I don't really know enough about our python code to go into too much detail on this.

== Git ==
The main thing: 
* Make the subject line concise but informative in its own right, ie. without having to refer to another commit or the changed code itself - I could pick 100 examples (including my own) where this hasn't happened, but I think, if we don't do anything else, this is the main one - because the subject line is quite often going to be the only thing we see when we're looking at the history of a project, so no more things like:
** {{c|5191: wip - a couple of minor fixes}} ({{commit|d8151422142}})
** {{c|Bug #5113 - Don't do that, do this}} ({{commit|830b5d93688}})
** {{c|Bug #4594 - Fix to "deliberate" mistake.}} ({{commit|2d05d6dcb6c0}})
** {{c|False is better than None here}} ({{commit|79b904904fe|qacontrol}})
**: Incidentally, in the interests of fairness/not bullying, I tried to pick one commit from each committer here which demonstrated not being "concise but informative", and I couldn't find a single one from [[User:Glyn|Glyn]], so I hereby name this rule "Do what Glyn does".

There are plenty of other git commit guidelines out there, but this is [http://chris.beams.io/posts/git-commit/ a good overview of how to write commit messages]. As the author points out: <span class="plainlinks">
[http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html This]
[http://www.git-scm.com/book/en/Distributed-Git-Contributing-to-a-Project#Commit-Guidelines has]
[https://github.com/torvalds/subsurface/blob/master/README#L82-109 all]
[http://who-t.blogspot.co.at/2009/12/on-commit-messages.html been]
[https://github.com/erlang/otp/wiki/writing-good-commit-messages said]
[https://github.com/spring-projects/spring-framework/blob/30bce7/CONTRIBUTING.md#format-commit-messages before]</span>.

His example (mostly drawn from other sources), which encompasses much of what he says, goes something like:
 Summarize changes in around 50 characters or less
 
 More detailed explanatory text, if necessary. Wrap it to about 72
 characters or so. In some contexts, the first line is treated as the
 subject of the commit and the rest of the text as the body. The
 blank line separating the summary from the body is critical (unless
 you omit the body entirely); various tools like `log`, `shortlog`
 and `rebase` can get confused if you run the two together.
 
 Explain the problem that this commit is solving. Focus on why you
 are making this change as opposed to how (the code explains that).
 Are there side effects or other unintuitive consequences of this
 change? Here's the place to explain them.
 
 Further paragraphs come after blank lines.
 
  * Bullet points are okay, too
 
  * Typically a hyphen or asterisk is used for the bullet, preceded
    by a single space, with blank lines in between, but conventions
    vary here
 
 If you use an issue tracker, put references to them at the bottom,
 like this:
 
 Resolves: #123
 See also: #456, #789
Briefly:

* Separate subject from body with a blank line
* Limit the subject line to 50 characters
* Capitalize the subject line
* Do not end the subject line with a period
* Use the imperative mood in the subject line
* Wrap the body at 72 characters
* Use the body to explain what and why vs. how

We add one extra rule to this, which conflicts with his "put references [to issues] at the bottom [of the message]":
* Prefix the subject line with a 'source' - bug number, user story number, ''something'' which provides a reference to where a fuller context for the change can be viewed.
** On the bug number front, we have a couple of different styles:
**# {{c|1234: The title of the commit}} - to represent {{bug|1234}}
**# {{c|Bug #4321 - The title of the commit}}
**: I prefer the former, if only because 50 chars is not very much to play with so the less boilerplate the better; there's 6 extra characters in there to play with.
**: Of course, this point may be moot now that we're moving to Yodiz, where the number could represent various things so we need the prefix of "us-" or "bg-" or whatever else it might be (I'm sure there's another one?).
**:* On that note, I say lowercase, eg. {{c|us-24: The title of the commit}}, rather than uppercase.