[webkit-reviews] review denied: [Bug 16537] Make date code faster by removing redundant calls : [Attachment 18008] Make msToDayInMonth slightly more readable and avoid recalculating msSince1970ToAbsoluteYear

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has denied Eric Seidel <eric at webkit.org>'s
request for review:
Bug 16537: Make date code faster by removing redundant calls
http://bugs.webkit.org/show_bug.cgi?id=16537

Attachment 18008: Make msToDayInMonth slightly more readable and avoid
recalculating msSince1970ToAbsoluteYear
http://bugs.webkit.org/attachment.cgi?id=18008&action=edit

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
-static inline int msToYear(double ms)
+static inline int msSince1970ToAbsoluteYear(double msSince1970)

I don't really agree that this change is cleanup.

In the date code, "ms" always means "msSince1970." Substituting "msSince1970"
makes the code read awkwardly for me. Also, I think it's a mistake to put
"msSince1970" in only select places -- that makes it seem as if there's a
difference between "ms" functions and "msSince1970" functions, when there's
not.

Perhaps it would be better just to add a comment at the top of the file stating
that "ms" means "ms since 1970."

I'm also not sure what you mean by the term "AbsoluteYear." What is an absolute
year? (/me imagines an ad for Absolut Vodka on the back of the New Yorker.)

+    static int minYear =
std::min(msSince1970ToAbsoluteYear(getCurrentUTCTime()),
maximumYearForDST()-27) ;

Need a space around the "-" here.

The checkMonth change looks good. I wish the function had a name that hinted
that it changed it arguments, but I can't think of a good one.

Not sure what to do with this patch. I guess I'll r- for the comments above.