Please vote on std.datetime

[Jonathan M Davis]

On Friday 10 December 2010 06:29:40 Jens Mueller wrote:
> Andrei Alexandrescu wrote:
> > Jonathan M. Davis has diligently worked on his std.datetime
> > proposal, and it has been through a few review cycles in this
> > newsgroup.
> > 
> > It's time to vote. Please vote for or against inclusion of datetime
> > into Phobos, along with your reasons.
> 
> I cannot say anything regarding the datetime module but I'd like to say
> something about the unittest module. First it is really great having
> these asserts with improved output.
> I do not understand why one needs assertOpCmp!"=="(). According to TDPL
> there should be <, <=, >=, and >. == will be rewritten using opEquals.
> So I think <= and >= are missing and I do not know why there is
> assertOpCmp!"==".
> Further I'll find the following implementation of assertOpCmp better.
> 
> immutable result = mixin("lhs" ~ op ~ "rhs");
> if(!result)
> {
> 	if(msg.empty)
> 		throw new AssertError(format("[%s] %s [%s] failed", lhs, op, rhs), 
file,
> line); else
> 		throw new AssertError(format("[%s] %s [%s] failed: %s", %lhs, op, rhs,
> msg), %file, line); }
> 
> It is shorter and gives the similar diagnostics than the current
> version. But I haven't tested the code above.
> The actual version is hard coded to lhs.opCmp(rhs). Better leave the
> decision lhs.opCmp(rhs) vs rhs.opCmp(lhs) to the compiler.
> 
> Not sure about the following but maybe having something spelled out like
> this assertLessThan, assertLessEqual, etc. is also nice.
> For assertEqual I'd like it to mirror opEqual similar to as I did it
> above for opCmp. So have something like assertOpEquals() and then maybe
> also the name assertEquals as a synonym. I have to admit I do not know
> how to integrate !=.

assertOpCmp!"<=" and assertOpCmp!">=" could certainly be added (I didn't think 
of adding them). <, >, and == are included because those are the three results 
that opCmp() can have. The othor comparison operators are built from that, but 
the point is to test opComp(). And in that vein, your changes would defeat that 
purpose. opCmp() needs to be specifically called here (especially because == does 
convert to opEquals(); opCmp() == 0 needs to be specificall testable). 
assertOpCmp() isn't so much intended to test that a value is less than or 
greater than but that the opCmp() function works correctly (which would then 
translate to less than and greater than and the like working correctly.).

On the other hand, assertEqual() is _not_ specifically testing opEqual(). It is 
simply testing for equality. So, it doesn't call opEquals() directly. If it did, 
it wouldn't work with primitive types and would probably run into problems due 
to the fact that == translates to the version of opEquals() which takes two 
parameters rather than the overridable one which takes one.

> Why the binary predicate?
> With the current version I can write something like
> assertEqual!"a != b"(1, 2)
> which I find strange. Why not only do the check for equality.
> I.e.
> if(!(mixin("actual" ~ op ~ "expected"))
> {
> 	if(msg.empty)
> 		throw new AssertError(format("assertEquals() failed: actual [%s],
> expected [%s].", actual, expected), file, line); else
> 		throw new AssertError(format("assertEquals() failed: actual [%s],
> expected [%s]: %s", actual, expected, msg), file, line); }
> 
> What's the benefit of the binary predicate? Clearly it makes it more
> flexible but I dislike allowing something like assertEqual!"a != b"(1,
> 2).

The predicate was suggested in review. std.algorithm.equal() has a predicate. 
This mirrors that. Since assertEqual() is intended to test for general equality 
rather than to specifically test opEquals(), it seemed to make good sense to me.

- Jonathan M Davis