Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove IComparisonOperators<TSelf, TSelf, bool> and IParsable<TSelf> from the IQuantity interface #1454

Closed
lipchev opened this issue Dec 13, 2024 · 3 comments

Comments

@lipchev
Copy link
Collaborator

lipchev commented Dec 13, 2024

Here is the current declaration:

    /// <summary>
    ///     An <see cref="IQuantity{TUnitType}"/> that (in .NET 7+) implements generic math interfaces for equality, comparison and parsing.
    /// </summary>
    /// <typeparam name="TSelf">The type itself, for the CRT pattern.</typeparam>
    /// <typeparam name="TUnitType">The underlying unit enum type.</typeparam>
#if NET7_0_OR_GREATER
    public interface IQuantity<TSelf, TUnitType>
        : IQuantity<TUnitType>
        , IComparisonOperators<TSelf, TSelf, bool>
        , IParsable<TSelf>
#else
    public interface IQuantity<in TSelf, TUnitType>
        : IQuantity<TUnitType>
#endif
        where TSelf : IQuantity<TSelf, TUnitType>
        where TUnitType : Enum

I think that the IComparisonOperators<TSelf, TSelf, bool> and IParsable<TSelf> declarations should be moved to the implementation section.

The motivation: reduce the number of methods / operators required for the "custom quantities": consider how many methods are added by moving HowMuch from : IQuantity to : IQuantity<HowMuch, HowMuchUnit>.

A more theoretical motivation is that by having it on the interface, it feels like we're expressing a requirement, as in "I need this method in order to be able to do something"- which is clearly not the case here.
IMO anyone requiring the generic constraint can use a combination of constraints:

public List<TQuantity> ParseQuantities<TQuantity>(string text) where TQuantity: IParsable<TQuantity>;
public List<TUnit> ParseUnits<TQuantity, TUnit>(string text) where TQuantity: IQuantity<TUnit>, IParsable<TQuantity>;

Note that repeating the declaration 180 times would add few KB to the size, but if that's a priority, we could introduce some IDefaultQuantity<TQuantity> interface and move all interface declarations that are shared by all quantities:

#if NET7_0_OR_GREATER
        IComparisonOperators<TSelf, TSelf, bool>,
        IParsable<TSelf>,
#endif
        IComparable,
        IComparable<TSelf>,
        IConvertible,
        IEquatable<TSelf>,
        IFormattable
@lipchev
Copy link
Collaborator Author

lipchev commented Dec 13, 2024

Note that repeating the declaration 180 times would add few KB to the size, but if that's a priority, we could introduce some IDefaultQuantity<TQuantity> interface and move all interface declarations that are shared by all quantities:

Hmm, weirdly enough, after removing it from the interface there was actually a decrease in size (~1KB).. (🤷 )

@angularsen
Copy link
Owner

Yeah that makes sense I think, moving interfaces to implementations instead of requiring all IQuantity implementors to add all the same stuff. As long as there isn't some IQuantity-based code that expects this or that, you will find out soone enough I guess 😄

@lipchev
Copy link
Collaborator Author

lipchev commented Dec 30, 2024

Fixed by #1459

@lipchev lipchev closed this as completed Dec 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants