Skip to content

Revert "Implicit operator removed"#17

Open
mcintyre321 wants to merge 1 commit intomasterfrom
revert-16-master
Open

Revert "Implicit operator removed"#17
mcintyre321 wants to merge 1 commit intomasterfrom
revert-16-master

Conversation

@mcintyre321
Copy link
Owner

Reverts #16

@mcintyre321
Copy link
Owner Author

@Paul-Williams I've just accepted, then reverted your PR!

I have just reviewed #10, which adds


        public static bool operator ==(ValueOf<TValue, TThis> a, object b)
        {
            return b is TThis other && a == other;
        }

        public static bool operator !=(ValueOf<TValue, TThis> a, object b)
        {
            return !(a == b);
        }
        

With those operators in place, and the implict cast method, we get the following results

void Main()
{
	Foo foo = Foo.From("asd");
	Foo2 foo2 = Foo2.From("asd");
	("asd" == foo).Dump(); //true
	("asd" == foo2).Dump(); //true
	(foo2 == foo).Dump(); //false 
}
class Foo : ValueOf<string, Foo> { }

class Foo2 : ValueOf<string, Foo2> { }

Does this solve your original issue with the implicit cast, or am I missing something?

@Paul-Williams
Copy link
Contributor

Paul-Williams commented May 6, 2021

This code does not seem to work as intended.
It breaks your equality test:
Assert.IsTrue(clientRef1 == "ASDF12345");

Additionally the test:
("asd" == foo).Dump(); //true
breaks, if inverted to:
(foo == "asd").Dump(); //false

It also still allows the comparison of two different ValueOf types, which although is not a bug, is a design choice I think goes against the idea of avoiding primitive obsession. It means you could still compare an EmailAddress to a UserName. This, I think is what this class is intended to avoid.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants