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

StringParam.* SizeIs and HasLengthBetween are congruent methods, but have inconsistent naming style #147

Closed
ndrwrbgs opened this issue Jan 26, 2021 · 2 comments · Fixed by #151
Labels

Comments

@ndrwrbgs
Copy link
Contributor

ndrwrbgs commented Jan 26, 2021

This is very minor, and would require a peek at if SizeIs is being consistent with Collection or some other method, but I noticed the inconsistency while trying to invoke a length limit on a string and figured I should at least mention it here :)

Thanks Daniel!

@danielwertheim
Copy link
Owner

So you want: string.HasLength because it aligns better with string.HasLengthBetween (which I agree on)?

There's collection.SizeIs perhaps it came from there at some point when things were IEnumerable<> which then matched the String's IEnumerable<char>

What do you prefer?
A) Add new and remove old and make a major
B) Add new and obsolete the old with comment it will be removed in future major?

I would go with (A). Be happy and move on with life so to speak 😀

@ndrwrbgs
Copy link
Contributor Author

Coming back to it a few days later, I'm actually thinking perhaps we keep both? Let me "talk out" my thoughts on the options to bounce the ideas off of you...

Options

  • +HasLength
    • PRO: Intellisense re:HasLengthBetween
    • PRO: Matches string.Length
    • CON: Two methods do the same thing (SizeIs)
  • -SizeIs +HasLength
    • See above
    • PRO: One method for one concept
    • CON: NOT consistent with the naming of the same operation on IEnumerable/others elsewhere in the library
    • CON: Breaking change
  • -HasLengthBetween +HasSizeBetween
    • PRO: Consistent in the library
    • CON: Does not match string.Length
  • +HasLength +HasSizeBetween
    • PRO: Best intellisense/discoverability
    • CON: 2 methods do the same thing in 2 cases (4 total methods, 2 'superfluous' as just synonyms)
    • CON: HasSizeBetween is not exposed for any other types in the library (not be a concern if folks think of strings as inherently different from, say, Lists, but is a concern if they do not)

I'm actually thinking I've seen libraries make synonyms (not the word I'm looking for) for APIs before, so maybe the fear of folks getting confused and asking "What's the difference between Size and Length" is less than I worry. If you agree on that, I suggest the final option to just add some synonyms!

P.S. As far as wants, I was trying to be as unopinionated as possible to let you have free reign, especially in case you intended it to be as-is. Just wanted to let you know of what I saw in case it felt important to you.

P.P.S. While I was writing the above that Size isn't a property on any ICollection or extension on IEnumerable or Array, I wonder if CountIs or LengthIs might match existing BCL naming better, but let's ignore that for now :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants