Author: Protiguous

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;

        }

    }
}