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 usages of 'dur' #485

Closed
wants to merge 1 commit into from
Closed

Remove usages of 'dur' #485

wants to merge 1 commit into from

Conversation

Abscissa
Copy link
Contributor

@Abscissa Abscissa commented Mar 9, 2012

This goes with dlang/druntime#174

@ghost ghost assigned jmdavis Mar 23, 2012
@andralex
Copy link
Member

@jmdavis what do you think?

@jmdavis
Copy link
Member

jmdavis commented Mar 23, 2012

As I explained in the associated druntime pull request, I'm against the idea.

A number of people seem to like the idea of having non-generic functions such as seconds and hours so that they can write code like seconds(5) or 5.seconds(). Personally, I don't really like the idea (especially 5.seconds()). It means using common words as free functions, potentially causing conflicts, and the names are nouns rather than verbs as functions normally should be. dur!"hours"(5) is generic and is unlikely to conflict with anything. But I'm not completely against it as long as dur sticks around, particularly since a number of people really like the idea, much as I don't.

However, I'm definitely against renaming dur to duration. It wasn't duration in the first place, because then it gets too long when you call it (e.g. duration!"hours"(5) or duration!"seconds"2")), so while the name dur is a bit ugly, it's the best abbreviation that I could come up with, and I think that the function name needs to be abbreviated to avoid being too long - particularly in complicated expressions. On top of that, changing it to duration would break code. We've been trying to reduce code breakage, and I don't think that renaming dur to duration is worth the breakage that it would cause.

I haven't closed this pull request or the druntime one, because I've been waiting to see if Nick would remove the changes to dur and whether anyone else would have anything to say, but neither has happened, and I'm tempted to just close them at this point.

@Abscissa
Copy link
Contributor Author

As (more or less) indicated in the inital comment, this pull request is
completly dependent on the "dur" aliases from (
dlang/druntime#174 ) being added.
ie:

alias dur!"minutes" minutes;
alias dur!"hours" hours;

Etc.

If those aliases are added, then this merely updates phobos to use those
aliases instead of dur where applicable.

As he states in that pull request (druntime #174) , Jonathan is strongly
opposed to the other aspect of that pull request: "dur -> duration". So I
assume that's probably not going to go through. But since he was the only
one to comment on it, and it was still open, I was waiting for either some
final "yes" or "no" confirmation or additional people commenting before
doing anything else.

If the idea of "dur -> duration" is completely dead, but the aliases are
deemed accepable, then I can replace druntime #174 with a new pull request
that only adds the aliases and nixes the "dur -> duration". At that point,
this pull request can then be re-evaluated. Most of the changes in this
pull request are merely replacing phobos's usage of 'dur!"minutes"' with the
proposed 'minutes' alias, etc. So 99% of it isn't strictly necessary, but my
thoughts were that the "minutes", "hours", etc aliases would be the new
"primary" way to create a duration, and using 'dur' directly would merely be
for the few cases where the genericness is needed.

(I hope that's not too rambling to make sence...)

@Abscissa
Copy link
Contributor Author

"However, I'm definitely against renaming dur to duration. It wasn't duration in the first place, because then it gets too long when you call it (e.g. duration!"hours"(5) or duration!"seconds"2")), so while the name dur is a bit ugly, it's the best abbreviation that I could come up with, and I think that the function name needs to be abbreviated to avoid being too long - particularly in complicated expressions. On top of that, changing it to duration would break code. We've been trying to reduce code breakage, and I don't think that renaming dur to duration is worth the breakage that it would cause."

My reasoning for "dur->duration" was that the "minutes"/"hours"/etc aliases would be even shorter still and would eliminate 90+% of direct uses of "dur". Therefore, the motivation for a shortened version of "dur" would be strongly mitigated, thus enabling us to go with the more readable (and more popular from what I can tell) "duration".

Now, if "dur -> duration" is completely vetoed (I don't know how these processes work), then I won't push on that point anymore and druntime #174 cn be closed. In that case, the question becomes "Ok, now what about the aliases?" Personally, I think the possible issues with name conflicts, at least in these cases, are much more trivial and uncommon than jmdavis feels that they are. FWIW, many people on the NG seemed to be very much in favor of the aliases.

@Abscissa
Copy link
Contributor Author

@brad Anderson: Yea. I'd been holding off on that out of hope that the
"dur->duration" could be kept. But it doesn't look likely that will happen,
so I'll make an alternate pull request with just the aliases.

@Abscissa
Copy link
Contributor Author

@ALL: With all this talk about "aliases", I do realize standard phobos style
is to avoid aliases. So I just want to point out that I've explained why I
feel it's justifiable in this case over here, just in case anyone missed it:

dlang/druntime#174

@jmdavis
Copy link
Member

jmdavis commented Mar 23, 2012

@Abscissa Yes, hours(2) and seconds(5) are shorter than dur!"hours"(2) and dur!"seconds"(5), but they also look like variables, and it puts common names into the closest thing that D has to a global namespace - and they're not generic at all, which is one of the main reasons that they weren't used in the first place. Now, having both dur and the separate functions allows you to deal with generic code appropriately, so it's less of an issue that way, but it also creates multiple ways to do the same thing.

I don't like seconds(5) at all, and I think that 5.seconds() is horrible, but there are definitely a number of folks who dislike dur, don't care about the genericity, and just want to be able to do seconds(5) - not to mention that some like the idea of using UFCS with it (e.g. 5.seconds()) for whatever reason - though honestly, I think that it's a good example of what's wrong with UFCS.

But regardless of whether we introduce seconds, hours, etc., I'm still very much against changing dur to duration, because I think that dur is more useable, and it's going to break code to make the change.

@DmitryOlshansky
Copy link
Member

Ehm if all these hours, seconds etc. were @Property function helpers, then we'd have amazingly nice syntax:
5.seconds
2.hours
but
1.hours
kind of less cool, still it looks as if we had custom literal, something that C++ folks killed a mamonth for to introduce into the language.
And I don't buy argument for namespace pollution of hours, seconds, msecs, nsecs.

Changing dur --> duration is going to break code for no real value, so is a no go, IMHO.

@jmdavis
Copy link
Member

jmdavis commented Mar 23, 2012

And I don't buy argument for namespace pollution of hours, seconds, msecs, nsecs.

The exact same thing was being argued with std.log and some of its choices in function names.

@DmitryOlshansky
Copy link
Member

The fact that we are arguing about it to me indicates one of two things:
a) our module system is no good, and we admit or seek ways to fix it
b) it's OK, but we are afraid/reluctant/don't bother to use it when the need arise

I'm still of the opinion it's more of b, then a. But indeed there are issues with modules, tons of them.

@jmdavis
Copy link
Member

jmdavis commented May 28, 2012

Even if we add the wrappers for dur, these changes are completely unnecessary, so I'm closing this pull request.

@jmdavis jmdavis closed this May 28, 2012
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.

4 participants