5
\$\begingroup\$

In my current game I have the need for values ranging from 0 to 1, or from -1 to 1. The values should never exceed this threshold, so instead of constantly validating the values I made a custom data type which essentially clamps a float to that threshold. I have been researching various ways of making it better, and I feel like what I have now is just about perfect for my uses. I decided against implementing IConvertible since all I'm really doing is wrapping a float with code I need; the value is still a float, and can be extracted and converted in the unlikely event it would need to be.

Is there anything I can improve on? Anything I'm doing wrong? I'd love to know!

P.S. You'll notice there's mention of a "SBloat", that's the same thing as a Bloat, just signed and ranging from -1.0 to 1.0. The code is almost identical.

using System;
using UnityEngine;

namespace ProjectBleak {
    /// <summary> 
    /// Represents an unsigned binary float ranging from 0 to 1.0. If the value exceeds this limit, it will be clamped to stay in range.
    /// </summary>
    [System.Diagnostics.DebuggerDisplay("{value}")]
    public struct Bloat : IEquatable<Single>, IFormattable {

        /// <summary> 0 </summary>
        public static readonly float MinValue = 0f;
        /// <summary> 1.0 </summary>
        public static readonly float MaxValue = 1f;

        private float value;

        /// <summary> Should an error be thrown when the value would be lower than 0 or higher than 1.0? </summary>
        public bool ErrorOnExceed { get; set; }



        /// <summary>
        /// Represents an unsigned binary float ranging from 0 to 1.0. If the value exceeds this limit, it will be clamped to stay in range.
        /// </summary>
        /// <param name="value">Value ranging from 0 to 1.0</param>
        /// <param name="errorOnExceed">Should an error be thrown when the value would be lower than 0 or higher than 1.0?</param>
        public Bloat(float value, bool errorOnExceed = false) {
            ErrorOnExceed = errorOnExceed;
            if (value < MinValue) {
                value = MinValue;
                if (ErrorOnExceed) {
                    Debug.LogError($"Bloat:: {value} exceeds minimum allowed value.");
                }
            }
            else if (value > MaxValue) {
                value = MaxValue;
                if (ErrorOnExceed) {
                    Debug.LogError($"Bloat:: {value} exceeds maximum allowed value.");
                }
            }
            this.value = value;
        }


        public override int GetHashCode() {
            unchecked {
                int hash = 17;
                hash = hash * 23 + value.GetHashCode();
                if (ErrorOnExceed) {
                    hash = hash |= 1 << 19;
                }
                else {
                    hash = hash |= 1 << 13;
                }
                return hash;
            }
        }


        public bool Equals(float other) {
            return value == other;
        }


        public override bool Equals(object obj) {
            if (obj is SBloat || obj is Bloat || obj is Single) {
                return Equals((float)obj);
            }
            return false;
        }


        public override String ToString() {
            return value.ToString();
        }

        public String ToString(IFormatProvider provider) {
            return value.ToString(provider);
        }

        public String ToString(String format) {
            return value.ToString(format);
        }

        public String ToString(String format, IFormatProvider provider) {
            return value.ToString(format, provider);
        }


        // Keep the Signed-to-Unsigned conversion explicit since we will be losing any negative number
        public static explicit operator Bloat(SBloat s) => new Bloat(s);
        public static implicit operator Bloat(Single f) => new Bloat(f);
        public static implicit operator float(Bloat b) => b.value;


        public static Bloat operator +(Bloat left, Single right) {
            return new Bloat(left.value + right, left.ErrorOnExceed);
        }
        public static Bloat operator -(Bloat left, Single right) {
            return new Bloat(left.value - right, left.ErrorOnExceed);
        }
        public static Bloat operator +(Bloat left, Bloat right) {
            return new Bloat(left.value + right.value, left.ErrorOnExceed);
        }
        public static Bloat operator -(Bloat left, Bloat right) {
            return new Bloat(left.value - right.value, left.ErrorOnExceed);
        }


        public static bool operator ==(Bloat left, Single right) {
            return left.value == right;
        }
        public static bool operator !=(Bloat left, Single right) {
            return left.value != right;
        }
        public static bool operator <(Bloat left, Single right) {
            return left.value < right;
        }
        public static bool operator >(Bloat left, Single right) {
            return left.value > right;
        }
        public static bool operator <=(Bloat left, Single right) {
            return left.value <= right;
        }
        public static bool operator >=(Bloat left, Single right) {
            return left.value >= right;
        }


        public static bool operator ==(Single left, Bloat right) {
            return left == right.value;
        }
        public static bool operator !=(Single left, Bloat right) {
            return left != right.value;
        }
        public static bool operator <(Single left, Bloat right) {
            return left < right.value;
        }
        public static bool operator >(Single left, Bloat right) {
            return left > right.value;
        }
        public static bool operator <=(Single left, Bloat right) {
            return left <= right.value;
        }
        public static bool operator >=(Single left, Bloat right) {
            return left >= right.value;
        }


        public static bool operator ==(Bloat left, Bloat right) {
            return left.value == right.value;
        }
        public static bool operator !=(Bloat left, Bloat right) {
            return left.value != right.value;
        }
        public static bool operator <(Bloat left, Bloat right) {
            return left.value < right.value;
        }
        public static bool operator >(Bloat left, Bloat right) {
            return left.value > right.value;
        }
        public static bool operator <=(Bloat left, Bloat right) {
            return left.value <= right.value;
        }
        public static bool operator >=(Bloat left, Bloat right) {
            return left.value >= right.value;
        }


        public static bool operator ==(Bloat left, SBloat right) {
            return left.value == right;
        }
        public static bool operator !=(Bloat left, SBloat right) {
            return left.value != right;
        }
        public static bool operator <(Bloat left, SBloat right) {
            return left.value < right;
        }
        public static bool operator >(Bloat left, SBloat right) {
            return left.value > right;
        }
        public static bool operator <=(Bloat left, SBloat right) {
            return left.value <= right;
        }
        public static bool operator >=(Bloat left, SBloat right) {
            return left.value >= right;
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! I rolled back your last edit. After getting an answer you are not allowed to change your code anymore. This is to ensure that answers do not get invalidated and have to hit a moving target. If you have changed your code you can either post it as an answer (if it would constitute a code review) or ask a new question with your changed code (linking back to this one as reference). Refer to this post for more information \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Aug 23 '18 at 20:29
  • 1
    \$\begingroup\$ Interesting type name... ('code bloat') \$\endgroup\$ – Pieter Witvoet Aug 24 '18 at 7:40
  • 1
    \$\begingroup\$ @PieterWitvoet It was either that or Bingle \$\endgroup\$ – user177952 Aug 24 '18 at 13:54
  • 1
    \$\begingroup\$ Heh, I see. Personally I'd go for BFloat, so the link with float is more obvious. Actually, binary seems to suggest only 2 possible values, so normalized or ranged might be more clear. \$\endgroup\$ – Pieter Witvoet Aug 24 '18 at 15:16
1
\$\begingroup\$

All in all it seems OK to me. I have these comments:

This test:

  Bloat a = new Bloat(0.5f);
  object b = new Bloat(0.3f);

  Console.WriteLine(a.Equals(b));

Throws an InvalidCastException

in:

public override bool Equals(object obj)
{
  if (obj is SBloat || obj is Bloat || obj is float)
  {
    return Equals((float)obj);  // Invalid Cast
  }
  return false;
}

The solution seems to be to cast to Bloat instead of float:

return Equals((Bloat)obj);

I don't understand the implementation of GetHashCode(). What do you gain from the factors and adds? Why not just return value.GetHashCode()?


You implement IEquatable<float> but not IEquatable<Bloat>? Without the latter, this example results in an unnecessary call to the Bloat-to-float operator:

  Bloat a = new Bloat(0.5f);
  Bloat b = new Bloat(0.3f);
  Console.WriteLine(a.Equals(b));

In the constructor you have the parameter errorOnExceed and the comment says that an error will be thrown if errorOnExceed == true and the value is out of range. I don't see that happen? I'm not sure, but IMO it seems that ErrorOnExceed should be a static member valid for all instances in an application.


In order to sort a List<Bloat> by calling List<Bloat>.Sort() or to use IEnumerable<Bloat>.OrderBy(b => b) you'll have to implement IComparable<Bloat>.


You implement for instance the +-operator for Bloat and float:

public static Bloat operator +(Bloat left, float right)
{
  return new Bloat(left.value + right, left.ErrorOnExceed);
}

this is fine and is used in this situation:

Bloat a = new Bloat(0.5f);
float b = 0.3f;

Bloat c = a + b;

But if you do this:

Bloat c = b + a;

it works through the Blot + Bloat operator with an overhead of calling the implicit float-to-Bloat operator for b. For optimization and symmetry you should consider to implement the float + Bloat operator. The same applies to the other operators for float as well as for SBloat.


You alternate between Single and float. For readability consider to use only one of them.

\$\endgroup\$
  • 1
    \$\begingroup\$ Thank you for your feedback! I have updated the code to reflect my changes. Some notes: The GetHashCode() method was my naive attempt at making the hash unique, but I never considered just using the existing hash for the float value. errorOnExceed simply throws an in-game error. You'll notice the Debug.LogError(...) within the constructor. Since the values are clamped this seems to be all I'll need. For now, at least. Alternating between Single and float was mainly done to match the colors for Single and Bloat - not a good reason, I know. \$\endgroup\$ – user177952 Aug 23 '18 at 20:35
1
\$\begingroup\$

Updated code:

using System;
using UnityEngine;

namespace ProjectBleak {
    /// <summary> 
    /// Represents an unsigned binary float ranging from 0 to 1.0. If the value exceeds this limit, it will be clamped to stay in range.
    /// </summary>
    [System.Diagnostics.DebuggerDisplay("{value}")]
    public struct Bloat : IEquatable<Bloat>, IEquatable<SBloat>, IEquatable<float>, 
                                                IComparable<Bloat>, IComparable<SBloat>, IComparable<float>, IFormattable {

        /// <summary> 0 </summary>
        public static readonly float MinValue = 0f;
        /// <summary> 1.0 </summary>
        public static readonly float MaxValue = 1f;

        private float value;

        /// <summary> Should an error be thrown when the value would be lower than 0 or higher than 1.0? </summary>
        public bool ErrorOnExceed { get; set; }



        /// <summary>
        /// Represents an unsigned binary float ranging from 0 to 1.0. If the value exceeds this limit, it will be clamped to stay in range.
        /// </summary>
        /// <param name="value">Value ranging from 0 to 1.0</param>
        /// <param name="errorOnExceed">Should an error be thrown when the value would be lower than 0 or higher than 1.0?</param>
        public Bloat(float value, bool errorOnExceed = false) {
            ErrorOnExceed = errorOnExceed;
            if (value < MinValue) {
                value = MinValue;
                if (ErrorOnExceed) {
                    Debug.LogError($"Bloat:: {value} exceeds minimum allowed value.");
                }
            }
            else if (value > MaxValue) {
                value = MaxValue;
                if (ErrorOnExceed) {
                    Debug.LogError($"Bloat:: {value} exceeds maximum allowed value.");
                }
            }
            this.value = value;
        }


        public override int GetHashCode() {
            return value.GetHashCode();
        }


        public override bool Equals(object obj) {
            if (obj is Bloat || obj is SBloat || obj is float) {
                return Equals((Bloat)obj);
            }
            return false;
        }

        public bool Equals(Bloat other) {
            return value == other;
        }

        public bool Equals(SBloat other) {
            return value == other;
        }

        public bool Equals(float other) {
            return value == other;
        }


        public int CompareTo(Bloat other) {
            return value.CompareTo(other);
        }

        public int CompareTo(SBloat other) {
            return value.CompareTo(other);
        }

        public int CompareTo(float other) {
            return value.CompareTo(other);
        }


        public override String ToString() {
            return value.ToString();
        }

        public String ToString(IFormatProvider provider) {
            return value.ToString(provider);
        }

        public String ToString(String format) {
            return value.ToString(format);
        }

        public String ToString(String format, IFormatProvider provider) {
            return value.ToString(format, provider);
        }


        // Keep the Signed-to-Unsigned conversion explicit since we will be losing any negative number
        public static explicit operator Bloat(SBloat s) => new Bloat(s);
        public static implicit operator Bloat(float f) => new Bloat(f);
        public static implicit operator float(Bloat b) => b.value;


        public static Bloat operator +(Bloat left, Bloat right) {
            return new Bloat(left.value + right.value, left.ErrorOnExceed);
        }
        public static Bloat operator -(Bloat left, Bloat right) {
            return new Bloat(left.value - right.value, left.ErrorOnExceed);
        }
        public static Bloat operator +(Bloat left, SBloat right) {
            return new Bloat(left.value + right, left.ErrorOnExceed);
        }
        public static Bloat operator -(Bloat left, SBloat right) {
            return new Bloat(left.value - right, left.ErrorOnExceed);
        }
        public static Bloat operator +(Bloat left, float right) {
            return new Bloat(left.value + right, left.ErrorOnExceed);
        }
        public static Bloat operator -(Bloat left, float right) {
            return new Bloat(left.value - right, left.ErrorOnExceed);
        }
        public static float operator +(float left, Bloat right) {
            return left + right.value;
        }
        public static float operator -(float left, Bloat right) {
            return left - right.value;
        }


        public static bool operator ==(Bloat left, float right) {
            return left.value == right;
        }
        public static bool operator !=(Bloat left, float right) {
            return left.value != right;
        }
        public static bool operator <(Bloat left, float right) {
            return left.value < right;
        }
        public static bool operator >(Bloat left, float right) {
            return left.value > right;
        }
        public static bool operator <=(Bloat left, float right) {
            return left.value <= right;
        }
        public static bool operator >=(Bloat left, float right) {
            return left.value >= right;
        }


        public static bool operator ==(float left, Bloat right) {
            return left == right.value;
        }
        public static bool operator !=(float left, Bloat right) {
            return left != right.value;
        }
        public static bool operator <(float left, Bloat right) {
            return left < right.value;
        }
        public static bool operator >(float left, Bloat right) {
            return left > right.value;
        }
        public static bool operator <=(float left, Bloat right) {
            return left <= right.value;
        }
        public static bool operator >=(float left, Bloat right) {
            return left >= right.value;
        }


        public static bool operator ==(Bloat left, Bloat right) {
            return left.value == right.value;
        }
        public static bool operator !=(Bloat left, Bloat right) {
            return left.value != right.value;
        }
        public static bool operator <(Bloat left, Bloat right) {
            return left.value < right.value;
        }
        public static bool operator >(Bloat left, Bloat right) {
            return left.value > right.value;
        }
        public static bool operator <=(Bloat left, Bloat right) {
            return left.value <= right.value;
        }
        public static bool operator >=(Bloat left, Bloat right) {
            return left.value >= right.value;
        }


        public static bool operator ==(Bloat left, SBloat right) {
            return left.value == right;
        }
        public static bool operator !=(Bloat left, SBloat right) {
            return left.value != right;
        }
        public static bool operator <(Bloat left, SBloat right) {
            return left.value < right;
        }
        public static bool operator >(Bloat left, SBloat right) {
            return left.value > right;
        }
        public static bool operator <=(Bloat left, SBloat right) {
            return left.value <= right;
        }
        public static bool operator >=(Bloat left, SBloat right) {
            return left.value >= right;
        }
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy