Opinions

Opinions

• Introduction
• Additional Guidelines
  • Prefer a file per code unit (class, struct, interface, etc.), except for delegates
  • Use the builder pattern (templated on Immutable collections) rather than a mutable class
  • Always ConfigureAwait(false) before await
  • Immutable types should implement IEquatable<T> and the == and != operators
  • Use HashCode rather than a custom GetHashCode() implementation
  • Use Throw members instead of throwing exceptions directly
  • Do not expose primitive types in public interfaces
  • Do not expose arrays in public interfaces
  • Do not expose nullable references in public interfaces
  • Check non-nullable reference types for null at interface boundaries
  • Parameter names must be whitelisted
  • Helpful ToString() overrides for public types
  • Avoid static constructors
• Changed Guidelines
  • Prefer to seal public classes
  • No 0 value for non-Flags public enums
  • Prefer byte sized enums

Introduction

Cesil mostly follows Microsoft's published framework design guidelines.

However, Cesil follows a few additional rules and modifies or discards some others. This page documents and justifies them.

Additional Guidelines

These are further restrictions on top of Microsoft's guidelines.

Prefer a file per code unit (class, struct, interface, etc.), except for delegates

Discourages extremely large files, and imposes consistency in structure. It is acceptable to split a code unit into multiple files if needed.

Use the builder pattern (templated on Immutable collections) rather than a mutable class

For thread safety, all instances actually used during reading and writing need to be immutable. Explicitly delimiting between "making a thing" and "the thing" facilitates this.

PublicInterfaceTests.Builders() checks that builders expose the expected interface.

Always ConfigureAwait(false) before await

Cesil is a library, and shouldn't care about the context any of its continuations and thus shouldn't capture SynchronizationContext. To do this, all await operations must be run against a Task or ValueTask that has had ConfigureAwait(false) called on it.

The only exception to this is if a consumer provided callback is invoked after an await. In this case, a consumer would reasonably expect the callback to happen in the same context where it was provided. There are currently no such exceptions in Cesil.

Rather than doing this directly, Cesil has AwaitHelper.ConfigureCancellableAwait(...) which does this while enabling better testing for cancellation purposes as well.

Immutable types should implement IEquatable<T> and the == and != operators

Immutable types conceptually have value semantics - implementing these methods allows users (and Cesil's own code) to benefit from those value semantics.

Use HashCode rather than a custom GetHashCode() implementation

Implementing GetHashCode() correctly is non-trivial, the built in class should be preferred.

Use Throw members instead of throwing exceptions directly

Exceptions should be rare, and occur in cases where performance isn't a concern. Additionally, throw can prevent some JIT optimizations from being made.

By using the Throw helper, we shrink some methods and enable some optimizations at the cost of imperceptibly slowing down exceptional passes.

Do not expose primitive types in public interfaces

Primitive types (int, bool, byte, etc.) moves the burden of correct interpretation and use onto the user, when a fair amount can be enforced by the type system.

For example, rather than taking or returning ints, using a ColumnIdentifier helps a user correctly interpret and use Cesil.

Exceptions can be made for IsXXX or HasXXX properties returning bool. Apply the IntentionallyExposedPrimitiveAttribute to suppress checks in PublicInterfaceTests.

Do not expose arrays in public interfaces

Arrays are mutable, which means correctly exposing arrays imposes protective copies.

Use IEnumerable<T>, (ReadOnly)Span<T>, (ReadOnly)Memory<T>, or ReadOnlySequence<T> as appropriate.

Do not expose nullable references in public interfaces

With C# 8's introduction of nullable reference types we can now distinguish between references that can and cannot be null.

Nulls are a perennial source of errors, and each nullable reference requires users write an explicit null check to avoid those errors. Accordingly, if at all possible reference types exposed by Cesil should be non-nullable.

For exceptions where a reference type truly is nullable (like passing back context objects, which are optional and thus object?), apply the NullableExposedAttribute attribute.

Check non-nullable reference types for null at interface boundaries

Despite C# 8's introduction of nullable reference types, users who have not enabled nullable reference checking can still pass null in places Cesil has documented that is illegal.

All public methods taking non-nullable reference type parameters must assert that the type is truly non-null as soon as reasonably possible.

Use Utils.CheckArgumentNull<T>(T, string) to implement these checks.

Parameter names must be whitelisted

As of C# 4 introduction of named parameters, parameter names are now part of a library's public interface. To prevent a proliferation of parameter names, all Cesil's paramters are white listed in PublicInterfaceTests.ParameterNamesApproved().

If it makes sense, use an already whitelisted name for any new parameters.

Helpful ToString() overrides for public types

To aid in debugging, all public types or types implementing public interfaces must have a helpful ToString() override.

This override must begin with the type name. This is checked in PublicInterfaceTests.HelpfulToString

Avoid static constructors

Static constructors (also known as Type Initializors) are a .NET feature that allows you to provide code that runs when a type is initialized. They're tempting to use when you need more control than can be easily put in member initializers.

.NET makes pretty strong guarantees about when static constructors will run, namely at:

• First access to any static or instance field of that type, or
• First invocation of any static, instance or virtual method of that type

In order to uphold these guarantees, the JIT will typically inject checks before every access that might trigger a type's static constructor. The overhead of these checks isn't worth it unless the stronger guarantees are actually needed, and I do not believe most code legitimately does. Accordingly, Cesil does not use static constructors.

Another way to state this, though one that is less familiar is "all classes must be BeforeFieldInit" - and in fact that is what Cesil tests for.

Changed Guidelines

These are changes to Microsoft's guidelines.

Some guidelines were laid down two decades ago, and don't make a ton of sense if you're starting from scratch. Others don't make much sense in the context of Cesil, and some I just personally disagree with (and, to be clear, I could be wrong).

Prefer to seal public classes

Inheritance as an extension point requires really careful planing, and the use cases that made more sense in the past can now be handled with extension methods or generics.

In the rare cases where a class has many logical extension points, and subclassing thus makes sense, apply IntentionallyExtensibleAttribute.

As an example, there is a single case of this in Cesil currently, and that's DefaultTypeDescriber - which implements "normal" .NET (de)serialization rules. These rules are complicated, and have about a dozen decision making points that a user might want to augment.

Internally, unsealed classes and inheritance are fine as implementation details.

No 0 value for non-Flags public enums

default and value types can sneak 0 values into unexpected places, so Cesil make a distinction between initialized and defaulted enumeration values. This only works if there is no 0 value for public enums.

Prefer byte sized enums

.NET is finding itself placed in more places where minimizing allocations and maximizing performance are paramount. While it is certainly unusual to create large allocations of Cesil's enums, either directly or indirectly, we cannot prohibit.

Therefore, if possible, prefer to add : byte to enum declarations.

Do not do this for a public enum if it is reasonable to expect that 7 or more variants will be needed in the future, resizing a public enum is a breaking change.