Flux Development Style Guide

This document explains the style guidelines for Flux development. It covers the naming conventions and commenting conventions for Flux code.

File & Class Headers

Insert something like the following header at the start of every file.

// ---------------------------------------------------------------------------------------
// Flux ~ Sheet metal CAD/CAM system
// Copyright (c) Metamation India.                                                === --
// ---------------------------------------------------------------------------      ==( )
// Part.cs                                                                        === --
// Implements the Part class, which is the generic unit of processing in
// Flux. A part may be a flat-sheet part, a bending part, a 5-axis laser part,
// a welding part (or many of these at the same time).
// ---------------------------------------------------------------------------------------

This header is from Flux.Core.dll, the other DLLs have a slightly different first line. Each top-level class in a file has a header like this, where the divider extends to column 100 (with the region open). The region header contains just two words like class Something or interface AnotherThing or enum YetAnother

namespace Flux;
#region class Part --------------------------------------------------------------------------------
/// <summary>Summary for class Part</summary>
/// More detailed documentation for class Part (a few lines, to a page or two even)
public class Part {
... many lines ...
}
#endregion

The class itself is divided into several segments and each segment has a header like this, extending to column 70.

public class Part {
   #region Properties -----------------------------------------------
   ...properties ...
   #endregion

   #region Methods --------------------------------------------------
   ... methods ...
   #endregion
}

Class Segments section below explains the possible segments in a class

Usings

The user defined namespaces should be added after all the system namespaces being used in a particular file.

For eg: Flux namespaces should come after the system namespaces as shown below.

using System.Windows;
using System.Windows.Controls;
using Flux.API.Bend;

Class Segments

The members of a class are organized into one or more segments. The order in which these segments are listed below is the preferred ordering of segments, and we explain here what should go into each segment.

Constructors

Public^[1] or protected constructors go here; private constructors go into the Implementation section. Static constructors should be private, and therefore should be in the implementation section. If there are static factory methods that create objects, then those should appear here in the Constructors section. For example:

class Pline {
   #region Constructors ---------------------------------------------
   /// <summary>Construct a Circle polyline, given the center and radius</summary>
   static Pline MakeCircle (Point2 center, double radius) {
   ... code ...
   }
   #endregion
}

Note that this appears in the constructors section only if this is static, and the return type is the enclosing class itself. In other words, it is effectively a constructor. The constructors are ordered in increasing complexity of parameter lists^[2].

Properties

The properties section lists all public properties, fields and events. We don’t distinguish between public properties and public fields, and where there is no particular need to use a property, we would recommend using a field instead. For example, consider this class:

public struct Point2 {
   public double X { get => mX; private set => mX = value; }
   double mX;

   public double Y { get => mY; private set => mY = value; }
   double mY;
}

This could safely be replaced by this:

public readonly struct Point2 {
   public readonly double X;
   public readonly double Y;
}

If, for some reason, we find it necessary to make X and Y properties (maybe we want to enforce some kind of range checking, or fire a property-changed event), this entails re-compilation of all the downstream code using Point2. That’s OK; we have the luxury of enforcing such re-compilation. If you are in doubt, consult senior programmer before using fields.

Properties are ordered alphabetically.

Methods

This lists all the public or protected methods of the type. Private methods go into the implementation section.

Overrides are treated in a special manner. If the root method of an override is part of Flux, then the method belongs here. For example, GridPunch.GenCode() method is listed here (since it implements ToolingItem.GenCode(), which is declared in Flux). On the other hand, GridPunch.ToString() method appears in the implementation section, since it overrides the Object.ToString() methods, which is declared outside Flux.

In some situations (as in implementing containers, or in WPF controls) one overrides a number of methods from a particular base class or interface. When doing this, it is a good idea to have a separate section titled something like UserControl implementation or IList implementation. Don’t do this if there are just one or two methods being overridden.

Methods appear in alphabetical order; methods with the same name are sorted in increasing order of parameter complexity.

Operators

This lists all public or protected type conversion operators, binary and unary operators. Private operators are moved into the implementation section^[3].

Implementation

This list all the private methods, as well as the overrides that got shunted here from the methods section.

Nested types

This lists all the nested types (including enumerations) that are declared within this class. Public types first, private types next. Types are listed in alphabetical order.

Private data

This lists private fields and properties. All instance fields come first, followed by all static fields. Within these sections they are listed in alphabetical order. There are some exceptions:

A field or private property that is used only in one method (for example, some kind of cache) can be placed immediately after that method.

public Rect2 GetBound () {
   if (mBound.IsEmpty) { ... compute bound ... }
   return mBound;
}
Rect2 mBound; // Place this here, rather than in the private data section

A field that is used as a backing field for a property is placed immediately after that property:

public double Area {
   get => mArea;
   set { mArea = value; PropertyChanged.Fire ("Area"); }
}
double mArea; // Place this here, rather than in the Private data section

Follow this rule in general. Extending this, one could infer that static data that is used only in one static constructor should appear immediately after that static constructor.

Naming Conventions

Class names: PlineIntersector

Methods: GetExtendedBounds (…)

Public fields & properties: MinHeight

Private instance fields & properties: mExtending

Private static fields and properties: sCharMap

Enumeration types: EClampType, ECorner, ECharacterSet

Interface types: IDisplayDevice, ICurlable, IFormattable

Use an m prefix for instance fields, and an s prefix for static fields, but no other type-specific (Hungarian) prefixes^[4].

Coding Conventions

Use a stable (global) path for Flux.API

Using a relative path will make it difficult for somebody who pulls your project to compile it. Also, it is best to checkout project files into a location closer to some drive root, rather than to your user-specific folders. So clone FChassis repo to W:/FChassis (where W: is work drive, created by "SUBST W: C:\Work")
Update .gitignore to ignore folders with temporary artifacts#

Sometimes temporary files will be created which need not be in repo. For example say intermediate code files are generated in Sample/Head1 and Sample/Head2 folders. These should be added to .gitignore, so they don’t keep showing up as 'Untracked files'. Even worse, we may forget and add the contents of these folders to the repository.
Don’t use the "private" keyword

Private is the default access specifier in most cases (except for interfaces). So we rarely use it, if ever, need to specify the 'private' keyword in front of any field, property or method.
Remove unnecessary usings

Very often, when you create a new CS file, Visual Studio will helpfully add a bunch of using statements at the top. Before you commit, remove the ones that are not actually being used to reduce clutter. You can right click on one of these and execute the cleanup action (you can do this for the entire solution or project in one go).
Use expression-bodies for trivial properties

Trivial properties that consist of a single expression should be replaced by a single expression body (reduce clutter).

Replace:
```
public Point3 StartPoint { get { return mStartPoint; } }
```
with:
```
public Point3 StartPoint => mStartPoint;
```
Use the [] notation to create collection-expressions succinctly

The creation of a new empty collection can be written very simply using the 'empty collection expression'.

Replace:
```
HashSet<int> mHashProgNo = new HashSet<int> ();
```
with:
```
HashSet<int> mHashProgNo = [];
```
Even if the collection is initialized with some elements, you can use this to simplify the code (both the statements below compile into exactly the same code).

Replace:
```
mPriority = new List<EKind> { EKind.Hole, EKind.Cutout, EKind.Notch, EKind.Mark };
```
with:
```
mPriority = [EKind.Hole, EKind.Cutout, EKind.Notch, EKind.Mark];
```
Prefer file-scoped namespaces

When all the contents of a file are contained in one namespace (the usual case), prefer a file scoped namespace - this reduces one level of indent for all the code in the file, and makes it more readable.

Replace:
```
namespace FChassis {
  public partial class SettingsDialogHostWindow : Window {
     ...
  }
}
```
with:
```
namespace FChassis;

public partial class SettingsDialogHostWindow : Window {
   ...
}
```
Note the empty line after namespace FChassis - this is a whitespace convention we follow.

Use expression-bodies where possible (extension of [expression-bodies])

Not just property getters, but property setters, methods, constructors, operators can all be rewritten as a single expression. This reduces clutter of open and close braces, and often converts 3 lines of code into one.

Constructors.

Replace:

public GCodeGenerator (Process process) {
   mProcess = process;
}

with:

public GCodeGenerator (Process process) => mProcess = process;

Method bodies.

Replace:

public static double D2R (this double degrees) {
   return degrees * (Math.PI / 180);
}

with:

public static double D2R (this double degrees) => degrees * Math.PI / 180;

Getters and Setters.

Replace:

public double this[int i, int j] {
   get { return A[i, j]; }
   set { A[i, j] = value; }
}

with:

public double this[int i, int j] { get => A[i, j]; set => A[i, j] = value; }

Operators.

Replace:

public static XForm4 operator * (XForm4 a, XForm4 b) {
   return a.MultiplyNew (b);
}

with:

public static XForm4 operator * (XForm4 a, XForm4 b) => a.MultiplyNew (b);

Remove dead code before committing

Git allows you to always go back to a previous version of code if you want to retrace your history. So there is no reason to keep dead code hanging around behind a comment. It creates a messy look for the code and promotes a 'broken window syndrome'. Remove dead code aggressively, and if some code needs to remain in for now, always mark it with a TODO comment like this:
```
 // TODO: Not sure if this alternative leads to better sequencing:
 // items = items.SortBy (a => a.Z).ThenBy (a => a.X).ToList ();
```
That way, it is clear that you mean to come back to this code later and experiment with it.
Remove empty documentation nodes before commit

When you type in /// to create a documentation node, Visual Studio fills it with empty nodes for each of the parameters, return value, exceptions etc. If you are not actually documenting these, remove them.

Replace:
```
/// <summary>
/// Get the ordinate direction sense for a vector
/// </summary>
/// <param name="toolingItem"></param>
/// <returns></returns>
/// <exception cref="InvalidCastException"></exception>
```
With:
```
/// <summary>Get the ordinate direction sense for a vector</summary>
```
Note that we are using the 'single line summary' Metamation convention.
Don’t create lambda expressions unnecessarily

Functions like Dispatcher.Invoke, Task.Run etc require objects of type Action<T> or just Action (with no parameters). If you already have a method that matches this signature, you can just directly pass it in - there is no need to create a wrapper lambda. So you can replace:
```
Dispatcher.Invoke (() => {
   mOverlay.Redraw ();
});
```
with:
```
Dispatcher.Invoke (mOverlay.Redraw);
```
You can often elide the type in a new() expression

If the type of the object being created can be inferred (either because it is a parameter, or it is being assigned to a typed variable), then the type can be elided from a new expression.

Replace:
```
XForm4 transMatrix = new XForm4 ();
```
with:
```
var transMatrix = new ();
```
An alternative form of this (that is also acceptable) is:
```
var transMatrix = new XForm4 ();
```
This works even if you need to pass parameters to the constructor.

Replace:
```
Point3 prev = new Point3 (tooling.X, tooling.Y, mClearance);
```
with:
```
Point3 prev = new (tooling.X, tooling.Y, mClearance);
```
Use ?. to automate null checks

The ?. operator can be used to call a method on an object only if it is not null. Thus, we can replace:
```
public void ClearDrawables () {
   if (mDrawables != null) mDrawables.Clear ();
}
```
with:
```
public void ClearDrawables () => mDrawables?.Clear ();
```
Replace 'as' and a null check with 'is'

This code uses 'as' to try casting the Curve object to an Arc3, and then does some work if that is not null.
```
Arc3 arc = segsList[0].Curve as Arc3;
if (arc != null) {
   if (arc.Start.EQ (arc.End))
      return true;
}
return false;
```
A simpler way is to to use an is operator that can do the check AND return a temporary of the cast type in one shot:
```
if (segsList[0].Curve is Arc3 arc) {
   if (arc.Start.EQ (arc.End))
      return true;
}
return false;
```
In fact, this code above can then be cleaned up even further by merging the nested if:
```
if (segsList[0].Curve is Arc3 arc && arc.Start.EQ (arc.End)) return true;
return false;
```
Even this can be simplified further. We are checking an expression (which is already a boolean) and returning true if that expression is true. So, we can simply write:
```
return segsList[0].Curve is Arc3 arc && arc.Start.EQ (arc.End);
```
Enumeration types always start with a capital E

Enumeration types are always named with a capital E at the start. Likewise, interface types are always named with a capital I at the start. Also, since these are always types, do not use the Type suffix for the name.

Replace:
```
enum MoveType { Arc, Line };
```
with:
```
enum EMove { Arc, Line };
```
Don’t repeat namespace prefixes in class names; avoid very long type names

There is a readability issue with very long type names. Code like this is too verbose:
```
arcStartPoint = FChassisUtils.MovePoint (arcStartPoint, startNormal, FChassisMachineSettings.Instance.MachiningToolClearance);
```
Here’s what is recommended:
```
arcStart = Utils.MovePoint (arcStart, startNormal, MCSettings.It.Standoff);
```
In particular, since we are working with the FChassis namespace, starting type names with FChassis is redundant.

Rename:
```
FChassis.FChassisMachineSettings FChassis.FChassisUtils
```
to:
```
FChassis.MCSettings FChassis.
```
Rename such types to keep the names more compact, without losing legibility. Some of this is subjective, but code with very long identifiers for types, variables etc feels hard to understand.
Avoid multiple blank lines in code

There should never be two or more successive blank lines anywhere in the code.
The canonical Singleton pattern

Metamation convention: Use 'It' as the property by which we get at the singleton object (rather than Instance).

Replace:
```
static MCSettings instance;
public static MCSettings Instance => instance ?? (instance = new MCSettings ());
```
with:
```
public static MCSettings It => mIt ??= new ();
static MCSettings mIt;
```
Note how we use the ??= operator to conditionally assign to an object only if it is not null. Also, we use these conventions:
- Name fields with an m prefix
- Place the private backing field of a property after the property, not before it
  
  The code above is almost fine, but is not thread-safe. If you want to be absolutely rigorous about it, here is the correct, thread-safe, performant way that does not lock. (See System.Lazy for more documentation on that type)
  public static MCSettings It => mLazy.Value; static readonly Lazy<MCSettings> mLazy = new (() => new ());
  The initialize for Lazy can be cryptic, here is the same code with type names for clarity:
  static readonly Lazy<MCSettings> mLazy = new Lazy<MCSettings> (() => new MCSettings ());
  The Lazy<T> takes a delegate that can be called to construct a T on demand.

Pattern matching, instead of multiple == checks

Code like this can be simplified:

bool ShouldOutputInfo (Tooling item) => item.Kind == EKind.Hole || item.Kind == EKind.Mark;

to this:

bool ShouldOutputInfo (Tooling item) => item.Kind is EKind.Hole or EKind.Mark;

For example, this code can be further simplified from:

if (MCSettings.It.ToolConfiguration == MCSettings.EHeads.Left ||
    MCSettings.It.ToolConfiguration == MCSettings.EHeads.Both)
       tooling.Head = 0;

to this:

if (MCSettings.It.Heads is MCSettings.EHeads.Left or MCSettings.EHeads.Both)
   tooling.Head = 0;

In this example, ToolConfiguration is renamed to Heads, since that is what it really is:

Heads = Left/Right/Both (it is less generic a name than ToolConfiguration).

Here is another simplification.

From:

if (toolingItem.IsCutout ()) blockType =
   (Utils.GetFlange (Workpiece, toolingItem) == EFlange.Top ||
    Utils.GetFlange (Workpiece, toolingItem) == EFlange.Bottom ? -1 : 1) * 3;

to:

if (toolingItem.IsCutout ()) blockType =
   Utils.GetFlange (Workpiece, toolingItem) is EFlange.Top or EFlange.Bottom ? -3 : 3;

We can also pattern match on ranges.

From:

if (thick >= 3.5 && thick < 4.5) { webPageNo = 8; flangePageNo = 7; }
if (thick >= 4.5 && thick < 5.5) { webPageNo = 10; flangePageNo = 9; }
if (thick >= 6.5 && thick < 7.5) { webPageNo = 14; flangePageNo = 13; }

to:

if (thick is >= 3.5 and < 4.5) { webPageNo = 8; flangePageNo = 7; }
if (thick is >= 4.5 and < 5.5) { webPageNo = 10; flangePageNo = 9; }
if (thick is >= 6.5 and < 7.5) { webPageNo = 14; flangePageNo = 13; }

Use of the 'is not' pattern to simplify code

This code:

while (targetItem != null && !(targetItem is ListBoxItem)) {
   targetItem = VisualTreeHelper.GetParent (targetItem) as FrameworkElement;
}

can be rewritten as:

while (targetItem is not null and not ListBoxItem)
   targetItem = VisualTreeHelper.GetParent (targetItem) as FrameworkElement;

Note also the convention we follow that a single statement as the body of a for / while loop is not surrounded by braces.

Using the ??= operator (compound assignment)

Using the ??= operator can avoid the typical pattern of 'check if not null before assignment'.

Replace:

E3Plane YNegPlane {
   get {
      if (mYNegPlane == null)
         mYNegPlane = Ents.OfType<E3Plane> ().Single (a => a.ThickVector.Z.EQ (1));
      return mYNegPlane;
   }
}
E3Plane mYNegPlane;

with:

E3Plane YNegPlane {
   get {
      mYNegPlane ??= Ents.OfType<E3Plane> ().Single (a => a.ThickVector.Z.EQ (1));
      return mYNegPlane;
   }
}
E3Plane mYNegPlane;

In fact, this very common pattern can be simplified even further to this:

E3Plane YNegPlane =>
   mYNegPlane ??= Model.Ents.OfType<E3Plane> ().Single (a => a.ThickVector.Z.EQ (1));
E3Plane mYNegPlane;

More use of collection expressions [] (similar to [collection-expressions])

Collection expressions are useful to replace:

double[] mControlDiameter = new[] { 14.7 };

with this:

double[] mControlDiameter = [14.7];

They can also be used to create nested collections.

Replace:

List<List<GCodeSeg>>> mDrawable = new List<List<GCodeSeg>> () {
   new List<GCodeSeg> (), new List<GCodeSeg> ()
};

with:

List<GCodeSeg>[] mDrawable = [[], []];

Also, in parameter lists. This code:

Lux.Draw (EDraw.Lines, new Point3[2] { pt, pt2 });

can be replaced with:

Lux.Draw (EDraw.Lines, [pt, pt2]);

Using the spread operator .. to build up lists

When the spread operator (..) is used within a collection initializer, it effectively creates a list out of the contents of that collection. This works with Enumerable<T> as well. Here’s some old code:
```
public List<Tooling> GetToolings4LHHead () {
   var toolingItems = Cuts.Where (a => a.Head == 0).ToList ();
   toolingItems = toolingItems.OrderBy (obj => Priority.IndexOf (obj.Kind)).ToList ();
   return toolingItems;
}
```
that we can simplify to this:
```
public List<Tooling> GetToolings4LHHead () =>
   [.. Cuts.Where (a => a.Head == 0).OrderBy (a => Priority.IndexOf (a.Kind))];
```
- We don’t need to capture the results of the filter (Where) into an intermediate list (in fact, that is inefficient). We can just pass that sequence directly to OrderBy, and make a list at the end.
- Note how the .. at the start of the collection expression replaces the sequence we have with the actual elements of that sequence. Without that, this expression would be a list of exactly one IEnumerable<T>. With this .., the enumerable gets replaced with the actual enumerated elements, forming the correct list.
Don’t check with List.Contains before calling List.Remove

This code is inefficient, since it will walk through the List twice (once for the Contains, once for the Remove).
```
if (itemsSource.Contains (droppedData)) {
   itemsSource.Remove (droppedData);
   ... more code ...
}
```
Instead, write:
```
if (itemsSource.Remove (droppedData)) {
   ... more code ...
}
```
This works, since Remove returns a bool (true if the item was found and removed).

string.SubString can usually be replaced with a range operator

Replace:

if (ncname.Length > 20) ncname = ncname.Substring (0, 20);
string trimStart = ncname.SubString (5);

with:

if (ncname.Length > 20) ncname = ncname[..20];
string trimStart = ncname[5..];

Writing local functions

Follow these conventions when writing local functions:

Place the local functions at the end of the corresponding scope, rather than at the beginning.

Otherwise they can disrupt the normal reading of the code.

Use a special comment format with ….. in it to delimit the local function.

Here is an example below:

public void Clear (EClear clear) {
   if (Set (EClear.BendTech)) Q.BendTech = null;
   if (Set (EClear.FoldTech)) Q.FoldTech = null;
   if (Set (EClear.CutTech)) Q.CutTech = null;
   if (Set (EClear.Drawing) && (Q.Model != null || Q.SurfaceModel != null)) Q.Dwg = null;
   if (Set (EClear.Model) && (Q.Dwg != null || Q.SurfaceModel != null)) Q.Model = null;
   if (Set (EClear.SurfaceModel) && (Q.Dwg != null || Q.Model != null)) Q.SurfaceModel = null;

   // Helper ...............................
   bool Set (EClear bit) => (clear & bit) != 0;
}

1. In this discussion, treat internal as public

2. We know this is rather vague; do your best.

3. Not sure if one even needs private operators.

4. There is a special variation of this m prefix rule. For classes that are designed to be serialized with the Au metadata system, field names that start with an underscore are implicitly treated as cached fields that are never serialized. This greatly simplifies writing the Metadata for that type; without this convention, one would have to list every such cached field and explicitly tag it as Cached so it does not get serialized.