Generated code doesn't implement some comparison operators

[bbrysh]

The id structs generated don't implement some comparison operators (<,>, <=, or >=.) The CLR cannot automatically call the CompareTo implementation from Equals(object) or from the base comparison operator implementations.

[andrewlock]

Thanks, I specifically didn't add those previously, as I didn't want them. For IDs, those comparisons don't make any sense. It seems reasonable to make optionally add them though I think

[bbrysh]

The optional route sounds good to me. This would support a scenario like someone is searching for an integer id using a binary search of a sorted list. It also helps me get rid of some SonarQube warnings :)

[SteveDunn]

@bbrysh - you could also implement IComparable yourself, as the classes are partial.

[bbrysh]

I could, but to me that defeats the purpose of using the library - not writing boilerplate code. Also, it's a good idea to treat IComparable like IEqualable. If you implement the IEqualable interface, you should override the equality operators. For IComparable, you should override the comparison operators. The templates already override the equality operators, so a precedent has been set.

[SteveDunn]

I would say that checking equality of IDs is much more common than sorting IDs. Granted, sorting could be useful, but whether or not that fits with a general purpose ID library, I'm not sure. I'm toying with the same idea in my library, Vogen, which is similar, but focused more on 'Valuen Objects' and how to ensure you can't create default/invalid instances through the use of code analysers and compilation errors. Trouble is, the next logical step would then be implicit operators to/from the underlying type, at which point, you risk losing type safety. https://github.com/SteveDunn/Vogen

…

On Tue, 8 Mar 2022, 18:05 bbrysh, ***@***.***> wrote: I could, but to me that defeats the purpose of using the library - not writing boilerplate code. Also, it's a good idea to treat IComparable like IEqualable. If you implement the IEqualable interface, you should override the equality operators. For IComparable, you should override the comparison operators. The templates already override the equality operators, so a precedent has been set. — Reply to this email directly, view it on GitHub <#56 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACAJ6C4FYJMD4OP2NG7VELU66JHJANCNFSM5PR4VCXA> . You are receiving this because you commented.Message ID: ***@***.***>

[bbrysh]

I don't believe that implicit operators would be needed as the Value property is the underlying typed value. I think having the overloaded comparison operators is a good choice as nobody knows all the cases where this library might be used. Since IComparable is an optional part of the ids generated, when the interface is generated, the operators could also be generated.

[MovGP0]

For IDs, those comparisons don't make any sense.

I disagree. Ids often need be sorted for some reason. Especially with ids of type byte, int, or long.

[timbze]

== and != is not mentioned in OP, but that's what I mostly need for ID. I'm doing it manually for now but sure would be great to have the option to generate those!

[cyril265]

I'd like to add that > and < operators for ids are needed if you want to implement cursor based pagination. Your query would be something like that: db.Entity.Where(e => e.Id > previousId).OrderBy(e => e.Id)

[andrewlock]

I'm slow to get to this, but the fundamental point of "if you implement IComparable you should implement the operators" is sound.

I've done a redesign of the library in this PR, and added the operators in that PR:

• Fundamental redesign to allow complete customisation #117

The main idea is to make the library much more maintainable while also giving people a mechanism to customise the generated IDs as much as they like. Which will make it easy to make changes like this in general.