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;
}
}
}
Like this:
Like Loading...
Related
Published by Protiguous
C# Software Developer, SQL Server DBA, Father, and seeker of Truth.
View all posts by Protiguous