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

ToQuantity without displaying the number #61

Closed
breyed opened this issue Jan 17, 2014 · 11 comments
Closed

ToQuantity without displaying the number #61

breyed opened this issue Jan 17, 2014 · 11 comments

Comments

@breyed
Copy link

breyed commented Jan 17, 2014

Provide a mechanism to optionally omit the number from the ToQuantity result. For example, "case".ToQuantity(5, QuantityOptions.OmitNumber) would be "cases", where OmitNumber would be a Flags enum.

@MehdiK
Copy link
Member

MehdiK commented Jan 17, 2014

Checkout the above pull request. I called the enum ShowQuantityAs. There is also a third option, Words, which turns the quantity to words.

Why a flags enum? I don't see a point in it?

@breyed
Copy link
Author

breyed commented Jan 17, 2014

I was thinking a flags enum so that options could be combined. Off hand, I can think of one use case for combining: whether to include an indefinite article. Others may think of more. Here's an iteration that tries combines the best of the suggestions so far:

[Flags] enum QuantityComponents { None=0, NumberAsWords=0x1, NumberAsDigits=0x02, IndefiniteArticle=0x04 };

"case".ToQuantity(3, QuantityComponents.None) -> cases
"case".ToQuantity(3, QuantityComponents.NumberAsWords) -> three cases
"case".ToQuantity(3, QuantityComponents.NumberAsDigits) -> 3 cases
"case".ToQuantity(3, QuantityComponents.NumberAsDigits | QuantityComponents.IndefiniteArticle) -> 3 cases
"case".ToQuantity(1, QuantityComponents.NumberAsDigits | QuantityComponents.IndefiniteArticle) -> a case
"case".ToQuantity(1, QuantityComponents.IndefiniteArticle) -> a case
"case".ToQuantity(3, QuantityComponents.IndefiniteArticle) -> cases

@MehdiK
Copy link
Member

MehdiK commented Jan 17, 2014

I like the IndefiniteArticle option; but I am still not sure I understand the need for flags. To me IndefiniteArticle kinda contradicts others. For example I am not sure how 3 cases in "case".ToQuantity(3, QuantityComponents.NumberAsDigits | QuantityComponents.IndefiniteArticle) is any more indefinite than the NumberAsDigits or NumberAsWords examples.

The contradiction is also apparent in "case".ToQuantity(1, QuantityComponents.NumberAsDigits | QuantityComponents.IndefiniteArticle) -> a case as NumberAsDigits should return 1 and not a.

In other words in "case".ToQuantity(3, QuantityComponents.NumberAsDigits | QuantityComponents.IndefiniteArticle) the second option is ignored and in "case".ToQuantity(1, QuantityComponents.NumberAsDigits | QuantityComponents.IndefiniteArticle) the first one, and thus my confusion around the need for flags.

What am I missing?

@breyed
Copy link
Author

breyed commented Jan 17, 2014

The rule for the IndefiniteArticle flag would be to include an indefinite article when the language permits one for the specified quantity. In English, an indefinite article is only permitted for a quantity of one. So for English QuantityComponents.NumberAsDigits | QuantityComponents.IndefiniteArticle is a way of saying, "If the quantity is one, include 'a'; otherwise, include the number as digits."

You're right that the flags overlap and sometimes have no effect. I see that as reflecting the non-linear nature of languages, just as Pluralize has no effect when applied to "sheep".

@MehdiK
Copy link
Member

MehdiK commented Jan 17, 2014

"In English, an indefinite article is only permitted for a quantity of one.". I don't think that's accurate.

AFAIK indefinite article is for when the object/article is unknown to the listener and could be about one or many of it and apparently "a", "an", "some" and "any" can be used for indefinite articles. More info on WikiPedia.

@breyed
Copy link
Author

breyed commented Jan 17, 2014

The things you don't learn in the process of coding! I certainly didn't have plural indefinite articles in mind, nor can I think of any use cases for them, but if the term "indefinite article" includes plural, it would be confusing to give a different semantic. However, since the info came from Wikipedia, I thought I should check it. I looked in my personal favorite, the Oxford English Dictionary, and what do you know? It says that only singular is included. I flagged the Wikipedia article and posted on the talk page so that hopefully an expert will weigh in.

MehdiK added a commit that referenced this issue Jan 20, 2014
Added ShowQuantityAs parameter to ToQuantity - #61
@MehdiK
Copy link
Member

MehdiK commented Jan 20, 2014

That makes sense. I like IndefiniteArticle and would like to add support for it to Humanizer; that said I am going to leave it out for now. I will close this issue with #62 that addresses this issue directly.

IndefiniteArticle is something that could be used in other APIs too. Added #63 to address that.

@MehdiK MehdiK closed this as completed Jan 20, 2014
@breyed
Copy link
Author

breyed commented Jan 20, 2014

For forward compatibility, it's important to determine the long-term API before shipping a version with support for omitting the number. As it stands now, the merged pull request uses a non-flags enum. Normally, changing a non-flags enum to a flags enum requires a breaking change. That's because the .NET Framework convention for enums is a singular noun for non-flags enums and a plural noun for flags enums. That way the type of enum is readily distinguishable.

ShowQuantityAs breaks from convention. while it flows nicely off the tongue, the downsides are that (a) since it's a verb phrase, it looks like a method and (b) it doesn't provide a hint as to whether its flags or non-flags. Looking ahead to the addition of a non-mutually-exclusive field, there could be further confusion.

@MehdiK
Copy link
Member

MehdiK commented Jan 20, 2014

I did consider the long-term API. TBH I am not sold on the flags enum. Flags enums are good when all or at least most options can be mixed while in this case they cannot; e.g. ShowQuantityAs.Numeric | ShowQuantityAs.Words is one strange mutation.

WRT the name, IMHO ShowQuantityAs is more easily understandable than QuantityOptions or QuantityComponents. I find the the latter two names a bit confusing in terms of what they offer while ShowQuantityAs tells the user what it's going to do.

Programs must be written for people to read, and only incidentally for machines to execute.
– Abelson and Sussman

:)

@robdmoore
Copy link
Contributor

FWIW I totally agree with @MehdiK here; a flags enum would be really weird and also fairly inconsistent with the rest of the API surface area inside of Humanizer.

I'd suggest that if multiple options were needed in the future then assuming there are only a few valid options that make sense (like described above) you could simply add them to the non-flags enum thus making it much clearer to understand for consumers of the library.

@breyed
Copy link
Author

breyed commented Jan 20, 2014

I agree that a flags enum that mixes mutually and non-mutually exclusive fields can be confusing. System.IO.FileMode is an example of an enum with mostly mutually exclusive fields. In that case, to handle the one case of fields that could be combined, an extra field OpenOrCreate was added. This makes for a good API.

In the ToQuantity case, there are two questions that a caller needs to answer: (1) What, if anything, should be shown for the number? (2) What, if anything, should be shown for the article? Possibly there are other questions, too, although nothing comes to mind, and you can only future-proof an API so far without making other compromises. Question one has three choices. If you broadened the article selection to include definite articles, the second question would also have three choices. Each set of three choices is independent of the other. This argues for two separate parameters. Since there is no need to tackle the article problem at this point, we can safely include the ShowQuantityAs parameter as originally suggested without jeopardizing future changes to the API.

Even though I've now drunk the non-flags enum Kool Aid, I'm not so sure about using a verb-phrase for the enum name - not that I'm sure about using a conventional, stuffy, starched-collar noun-phrase, either. If I may be more than a trifle dramatic, there is an epic cosmic trade-off between the good of each individual method, which wants to tune itself to its ideal nuanced personality, which struggles against the grander assimilated API, which craves consistency and order, striving to keep its ranks regimented when soldier-methods from widely different backgrounds line up together on the coding battlefield.

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

No branches or pull requests

3 participants