HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Struct implementation of immutable Maybe<T> monad

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
structimmutablemaybeimplementationmonad

Problem

I have developed the following implementation of an immutable struct Maybe monad for use in my game project; as an practical exposition on monads to present to my local User Group; and an a demonstration of the use of Static Code Analysis for verification in conjunction with TDD. I am interested in any and all comments on coding style, and completeness and correctness of the implementation.

The base Maybe class:

```
/// An immutable value-type Maybe<T> monad.
/// The base type, which can be either a class or struct type,
/// and will have the Equality definition track the default for the base-type:
/// Value-equality for structs and string, reference equality for other classes.
///
/// Being a value-type reduces memory pressure on .
///
/// Equality tracks the base type (struct or class), with the further proviseo
/// that two instances can only be equal when is true
/// for both instances.
///
public struct Maybe : IEquatable> {
#region static support for IEquatable>
static Maybe() {
_valueIsStruct = typeof(ValueType).IsAssignableFrom(typeof(T));
_equals = ValueIsStruct ? (Func)ValEquals
: (Func)RefEquals;
_nothing = new Maybe();
}
static bool ValueIsStruct { [Pure]get {return _valueIsStruct;} } static bool _valueIsStruct;
static readonly Func _equals;
[Pure]private static bool ValEquals(T lhs, T rhs) { return lhs.Equals(rhs); }
[Pure]private static bool RefEquals(T lhs, T rhs) {
return typeof(string).IsAssignableFrom(typeof(T)) ? lhs.Equals(rhs)
: object.ReferenceEquals(lhs, rhs);
}
#endregion

public static Maybe Nothing {
[Pure]get { Contract.Ensures( ! Contract.Result>().HasValue);

Contract.Assume( ! _nothing.HasValue );
return _nothing;
}
} static Maybe _nothing;

///Create a new Maybe<T>.
public Maybe(T value) : this() {
value.ContractedNotNull("value");

_value = value;

Solution

Your code formatting is mind-blowing. It is so unconventional, that it looks chaotic (even though there is probably a system to it). I had a really hard time reviewing it, even though the business logic itself is fairly simple. I think you should use common .Net coding style, if you want to share your code with other .Net developers. Otherwise they will have a really hard time reading it (which is probably part of why your question did not get much attention).


Value-equality for structs and string, reference equality for other classes

Both value types and reference types have Equals(object) method, why can't you use it in both cases? By default it will call ReferenceEquals internally for reference types anyway. But if I ever want to override the default equality implementation in my T class, I will be able to do it. Your current implementation does not allow it by enforcing reference comparison. You could also take it one step further and check if T implements IEquatable interface, and if it does - call IEquatable.Equals instead, again, for both value and reference types.

Your equality methods implementations break some of the common conventions.

-
Two null objects should be equal to each other. Object.Equals(null, null) returns true. ((int?)null).Equals((float?)null) returns true. Etc. Its generally advised to follow this behavior in custom equality implementations, because it's the behavior people are expecting.

-
Your NotEquals() method should return the same result as !Equals(). If it does, then you should use !Equals() internally instead of !this.HasValue || !other.HasValue || .... If it doesn't, then you are doing something wrong. I'm also not sure why would you need NotEquals() method in the first place, it just bloats the API. The consumers of your class should be perfectly capable of applying ! operator to Equals method themselves.


return Bind(v => v.ToString()).Extract("");

Empty string is not very informative. I would rather have "Nothing: " + typeof(T) instead.


obj as Maybe?

is a lot slower than if (obj is Maybe) { (Maybe)obj } and not much of an improvement when it comes to readability. So you probably should not use as operator with nullables. Unless you are actually expecting a nullable, ofc.

What looks fishy to me is that your monad is an actual data type. I can't imagine a scenario where I would want to hold a reference to a monad, or, even worse, a collection of monads. Monad is an abstraction of computation: you construct a monad chain, you execute it and then you garb the result of the computation. That's it. Even if you want to hold a reference to a chain for some reason, that's what delegates are for. If you use older versions of C#, the basic implementation of Maybe monad is a two-liner:

static TResult Maybe(this TInput source, Func projector)
    where TResult : class where TInput : class
{
    if (source == null) return null;
    return projector(source);
}


In later versions of C# you can just use ?. operator instead. Maybe monad is supposed to be simple. As simple as a regular null check. Yet you made it look really complex for a reason, that is unclear to me.

Code Snippets

static TResult Maybe<TInput, TResult>(this TInput source, Func<TInput, TResult> projector)
    where TResult : class where TInput : class
{
    if (source == null) return null;
    return projector(source);
}

Context

StackExchange Code Review Q#119995, answer score: 6

Revisions (0)

No revisions yet.