(2010/08/17 20:33), Lars Tandle Kyllingstad wrote:
Looks good!
I just have a few small suggestions:
1. I'd replace fromMseconds and fromUseconds with fromMilliseconds and
fromMicroseconds. (Mseconds is, strictly speaking, megaseconds...) And
mseconds->milliseconds, useconds->microseconds, etc.
Are not you worried about a function name getting longer?
(I completely agree that Mseconds looks like megaseconds. :) )
Hmm, maybe I'll try to add this rather:
alias seconds sec;
alias milliseconds msec;
alias microseconds usec;
2. The opOpAssign() functions should return 'this'. Also, you can
combine them into one:
Ticks opOpAssign(string op)(Ticks t)
if (op == "+" || op == "-" || op == "*" || op == "/")
{
mixin("value "~op~"= t.value;");
return this;
}
They should probably return by ref as well, but bug 2460 prevents that.
I cannot apply all.
As for it, a dimension occasionally changes.
[s] + [s] == [s]
[s] - [s] == [s]
but
[s] / [s] == [%]
[s] * [s] == [s^2]
In addition, the calculation between the values that dimensions are
different needs attention.
[s] + [no dimension] -> x
[s] - [no dimension] -> x
[s] * [no dimension] -> [s]
[s] / [no dimension] -> [s]
I think that avoidance of coexisting is better when I consider a document.
Bug 2460 is troublesome.
It can become hotbeds of the bug if return rvalue...
I make an effort to comply with your advice as possible as.
3. The opBinary() functions can also be combined like that, or you can
even do:
Ticks opBinary(string op)(Ticks t)
{
auto lhs = this;
return lhs.opOpAssign!op(t);
}
I think that's a pretty nice idiom.
-Lars
It's interesting.
I think I want compiler to generate it automatically.
_______________________________________________
phobos mailing list
phobos@puremagic.com
http://lists.puremagic.com/mailman/listinfo/phobos
