Class simulating a circuit pin (follow-up)

Question

In a previous post I asked for a review on some code I wrote for an open-source project of mine. After the review I made some changes and I wanted to ask you for another round. This time I'm not putting all the comments in the code because I'm still working on them.

Other than that, I'd ask you guys to check and indicate everything that in your opinion is wrong, could be better, or whatever the case may be. Be merciless! :)

Pin.cs

/// <summary>
/// A pin used in the various logical gates.
/// </summary>
public class Pin : Observable<Pin> //, IComparable<Pin>, IComparable<int>
{
    #region variables
    private PinValue _value;
    private string _code;
    private string _label;
    #endregion

    #region properties
    /// <summary>
    /// Get the ID of the Pin.
    /// </summary>
    public Guid Id
    {
        get;
        private set;
    }

    /// <summary>
    /// Get or Set the value of the Pin.
    /// </summary>
    public PinValue Value
    {
        get
        {
            return _value;
        }
        set
        {
            _value = value;
            Notify(this);
        }
    }

    /// <summary>
    /// Get or Set the code of the Pin.
    /// </summary>
    public string Code
    {
        get
        {
            return _code;
        }
        set
        {
            _code = value;
            Notify(this);
        }
    }

    /// <summary>
    /// Get or Set the label of the Pin.
    /// </summary>
    public string Label
    {
        get
        {
            return _label;
        }
        set
        {
            _label = value;
            Notify(this);
        }
    }
    #endregion

    #region constructors
    public Pin()
    {
        Id = Guid.NewGuid();
        Value = new PinValue(null);
        Code = Id.ToString();
        Label = null;
    }

    public Pin(PinValue value)
        : this()
    {
        Value = value;
    }

    public Pin(string code)
        : this()
    {
        Code = code;
    }

    public Pin(PinValue value, string code)
        : this(value)
    {
        Code = code;
    }

    public Pin(string code, string label)
        : this(code)
    {
        Label = label;
    }

    public Pin(PinValue value, string code, string label)
        : this(value, code)
    {
        Label = label;
    }
    #endregion

    #region methods
    public override string ToString()
    {
        return string.Format(
            "[Id: {0} | Code: {1} | Label: {2} | Value: {3}]",
            Id,
            Code,
            Label,
            Value);
    }

    public override int GetHashCode()
    {
        // Id is the only immutable value of this object
        return Id.GetHashCode();
    }

    public override bool Equals(object obj)
    {
        return obj != null 
            && (obj is Pin) 
            && Equals(obj as Pin);
    }

    public bool Equals(Pin otherPin)
    {
        if (otherPin == null)
        {
            return false;
        }

        return Id == otherPin.Id
            && Code == otherPin.Code
            && Value == otherPin.Value
            && Label == otherPin.Label;
    }
    #endregion
}

PinValue.cs

public struct PinValue 
{
    private LogicalValue _value;

    /// <summary>
    /// The possible values a Pin can have.
    /// </summary>
    public enum LogicalValue
    {
        Unknown,
        Low,
        High
    }

    public LogicalValue Value
    {
        get
        {
            return _value;
        }
    }

    public PinValue(LogicalValue value)
    {
        _value = value;
    }

    public PinValue(bool? value)
    {
        if (value == null)
        {
            _value = LogicalValue.Unknown;
        }
        else if (value == true)
        {
            _value = LogicalValue.High;
        }
        else 
        {
            _value = LogicalValue.Low;
        }
    }

    public override bool Equals(object obj)
    {
        return obj != null
            && obj is PinValue
            && Equals((PinValue)obj);
    }

    public bool Equals(PinValue obj)
    {
        return obj.Value == Value;
    }

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

    public override string ToString()
    {
        return string.Format("{0}", Value);
    }

    public static bool operator <(PinValue x, PinValue y)
    {
        return x.Value < y.Value;
    }

    public static bool operator <=(PinValue x, PinValue y)
    {
        return x.Value <= y.Value;
    }

    public static bool operator >(PinValue x, PinValue y)
    {
        return x.Value > y.Value;
    }

    public static bool operator >=(PinValue x, PinValue y)
    {
        return x.Value >= y.Value;
    }

    public static bool operator ==(PinValue x, PinValue y)
    {
        return x.Value == y.Value;
    }

    public static bool operator !=(PinValue x, PinValue y)
    {
        return x.Value != y.Value;
    }

    public static bool operator true(PinValue val)
    {
        return val.Value == LogicalValue.High;
    }

    public static bool operator false(PinValue val)
    {
        return val.Value == LogicalValue.Low;
    }

    public static PinValue operator &(PinValue x, PinValue y)
    {
        if (x.Value == LogicalValue.High 
            && y.Value == LogicalValue.High)
        {
            return new PinValue(LogicalValue.High);
        }
        else if (x.Value == LogicalValue.Low 
                 || y.Value == LogicalValue.Low)
        {
            return new PinValue(LogicalValue.Low);
        }

        return new PinValue(LogicalValue.Unknown);
    }

    public static PinValue operator |(PinValue x, PinValue y)
    {
        if (x.Value == LogicalValue.High
            || y.Value == LogicalValue.High)
        {
            return new PinValue(LogicalValue.High);
        }
        else if(x.Value == LogicalValue.Low
                && y.Value == LogicalValue.Low)
        {
            return new PinValue(LogicalValue.Low);
        }

        return new PinValue(LogicalValue.Unknown);
    }

    public static PinValue operator ^(PinValue x, PinValue y)
    {
        if (x.Value != LogicalValue.Unknown
            && y.Value != LogicalValue.Unknown)
        {
            if (x.Value != y.Value)
            {
                return new PinValue(LogicalValue.High);
            }

            return new PinValue(LogicalValue.Low);
        }

        return new PinValue(LogicalValue.Unknown);
    }

    public static PinValue operator !(PinValue x)
    {
        PinValue result;

        if (x.Value == LogicalValue.Low)
        {
            result = new PinValue(true);
        }
        else if (x.Value == LogicalValue.High)
        {
            result = new PinValue(false);
        }
        else
        {
            result = new PinValue(null);
        }

        return result;
    }

    public static implicit operator PinValue(bool? val)
    {
        return new PinValue(val);
    }

    public static implicit operator bool(PinValue val)
    {
        return val.Value == LogicalValue.High;
    }

    public static implicit operator bool?(PinValue val)
    {
        if (val.Value == LogicalValue.High)
        {
            return true;
        }
        else if (val.Value == LogicalValue.Low)
        {
            return false;
        }

        return null;
    }
}

Answer 1

#region variables
private PinValue _value;
private string _code;
private string _label;
#endregion

They're not variables, they're fields. But the annoying part isn't the miswording - it's #region. Get rid of them.

You have this Id auto-property:

/// <summary>
/// Get the ID of the Pin.
/// </summary>
public Guid Id
{
    get;
    private set;
}

And then this comment:

// Id is the only immutable value of this object

From the outside, it's true. But the Id isn't immutable - it's an auto-property with a private setter, which means the class can change it whenever it feels like. C# 6.0 introduced actually immutable auto-properties; if you're not using C# 6.0, this is what an immutable property looks like:

private readonly int _id;
public int Id { get { return _id; } }

Notice how the backing field being right next to the associated public member defeats the purpose of #region, and how it puts the related stuff right next to each other ;-)

---

I would take the LogicalValue enum out of the enum. Maybe it's just me not liking nested types, but I find this uselessly verbose:

var pin = new Pin(PinValue.LogicalValue.Low);

---

This is starting to be very much about personal preference, but I find this:

public string Label
{
    get
    {
        return _label;
    }
    set
    {
        _label = value;
        Notify(this);
    }
}

Looks better like this:

public string Label
{
    get { return _label; }
    set
    {
        _label = value;
        Notify(this);
    }
}

The getter is straightforward; vertically extending only the setter puts some emphasis on it and its Notify behavior.