Skip to content

Add support for comparisons #4

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mediocresurgeon
Copy link

Adds the following methods:
Argument.GreaterThan()
Argument.GreaterThanOrEqualTo()
Argument.LessThan()
Argument.LessThanOrEqualTo()

…lTo()`, `Argument.LessThan()`, and `Argument.LessThanOrEqualTo()`.
@ashmind
Copy link
Owner

ashmind commented Feb 24, 2021

The technical work looks great, but I wonder if use cases (not already covered by Positive and similar checks) are common enough.
What are some use cases you have in mind for this one?

@mediocresurgeon
Copy link
Author

The technical work looks great, but I wonder if use cases (not already covered by Positive and similar checks) are common enough.
What are some use cases you have in mind for this one?

One real-world example might be String.Substring(int, int).

With the new functionality, we would be able to express the boundary conditions like so:

public string Substring (int startIndex, int length)
{
    Argument.PositiveOrZero(nameof(startIndex), startIndex); // or Argument.GreaterThanOrEqualTo(nameof(startIndex), startIndex, 0);
    Argument.LessThan(nameof(startIndex), startIndex, this.Length);
    
    Argument.PositiveOrZero(nameof(length), length); // or Argument.GreaterThanOrEqualTo(nameof(length), length, 0);
    Argument.LessThanOrEqualTo(nameof(length), length, this.Length - startIndex);
    
    // Implementation
}

@ashmind
Copy link
Owner

ashmind commented Feb 27, 2021

Thanks! Yep I was thinking of something like that or Array.CopyTo and similar.
But feels like there are not that many of those?

Also just remember one other thought I had when I considered similar methods for v1:
I feel in those specific examples general Less/Greater might not give a clear enough error.

E.g Argument.LessThan(nameof(startIndex), startIndex, this.Length); will tell you "Value cannot be greater than or equal to 10", which does explain why exactly, or what 10 is. What would be good instead is a custom exception message, e.g. "Start index cannot exceed string length (10)".

Similarly, instead of Argument.LessThanOrEqualTo(nameof(length), length, this.Length - startIndex); we might consider something like "Substring of length 10 starting at index 5 ends at index 15, which is located beyond the end of the string (10)" (that might be too verbose, first wording I could think of).

You could argue the same point about e.g. Positive... methods.
But I feel 0 is kind of special -- e.g. for length getting "Value must be positive" seems more clear on its own than "Value must be less than 17" (what's 17?).

@mediocresurgeon
Copy link
Author

You're right--the proposed exception message might be adequate for constants, but it doesn't meaningfully communicate how to untangle a more dynamic situation (eg "startIndex plus length indicates a position not within this instance").

How would you feel about including an overload for these new methods which allows the exception message to be specified?

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