namespace TestCode { using System; /// <summary>When to use fields, getters, properties, and/or setters.</summary> /// <remarks>Totally open to suggestions to showcase wrong/right patterns.</remarks> public class WrongBetterRight { /// <summary>Bad.</summary> public class VersionA { /// <summary>Totally exposed public field. No protection.</summary> public Int32 ID; /// <summary>getter that 'does' something. Should be a method.</summary> public Int32 CalculateID => ( this.ID / 2 ) + ( this.ID / 2 ); /// <summary>Okay, but still a getter.</summary> public Int32 GetID => this.ID; /// <summary>ctor.</summary> /// <param name="id"></param> public VersionA( Int32 id ) => this.ID = id; /// <summary>*poof* your app just blew up.</summary> /// <param name="newid"></param> public Boolean ChangeID( Int32 newid ) { this.ID = newid; return true; } } public class VersionB { /// <summary>Better than a public non-readonly field. Still vulnerable to a future change to property.</summary> public readonly Int32 ID; /// <summary>Only returns value. Okay.</summary> public Int32 GetID => this.ID; /// <summary>ctor. No checks yet.</summary> /// <param name="id"></param> public VersionB( Int32 id ) => this.ID = id; /// <summary>Contrived. 'Does' something, so a method is okay.</summary> /// <returns></returns> public Int32 CalculateID() => ( this.ID / 2 ) + ( this.ID / 2 ); /// <summary>Prevents this method from changing ID. Which the compiler would prevent anyways. Not as easy to refactor (what 'value'??).</summary> /// <param name="newid"></param> /// <returns></returns> public Boolean ChangeID( Int32 newid ) => throw new InvalidOperationException( "value is not allowed to be changed." ); } public class VersionC { /// <summary>Best way. Future proof. Property hides away field.</summary> public Int32 ID { get; } /// <summary>ctor. Checks incoming value.</summary> /// <param name="id"></param> public VersionC( Int32 id ) { this.ValidateValue( id ); this.ID = id; } private void ValidateValue( Int32 id ) { ThrowIfValueTooLow(); ThrowIfValueTooHigh(); void ThrowIfValueTooLow() { const Int32 minimumAllowedValue = 1; if ( id < minimumAllowedValue ) { //Exception message wording could better. But easy to refactor. throw new ArgumentOutOfRangeException( nameof( id ), id, $"The value `{id}` is too low for {nameof( this.ID )}." ); } } void ThrowIfValueTooHigh() { const Int32 maximumAllowedValue = 10; if ( id > maximumAllowedValue ) { //Exception message wording could better. But easy to refactor. throw new ArgumentOutOfRangeException( nameof( id ), id, $"The value `{id}` is too high for {nameof( this.ID )}." ); } } } /// <summary>Future proof. Doesn't expose <see cref="ID" /> directly. Easy to refactor.</summary> /// <returns></returns> public Int32 CalculateID() => ( this.ID / 2 ) + ( this.ID / 2 ); /// <summary>prevents this method from changing <see cref="ID" />. which the compiler would prevent anyways. but easier to refactor <see cref="ID" />.</summary> /// <param name="newid"></param> /// <returns></returns> public Boolean ChangeID( Int32 newid ) => throw new InvalidOperationException( $"{nameof( this.ID )} is not allowed to be changed." ); /// <summary>Valid. Better than a plain getter? Debatable. This is more future proof though.</summary> public Int32 GetID() => this.ID; } } }