[Webkit-unassigned] [Bug 16537] Make date code faster by removing redundant calls

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Dec 20 11:01:07 PST 2007


http://bugs.webkit.org/show_bug.cgi?id=16537


ggaren at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #18008|review?                     |review-
               Flag|                            |




------- Comment #7 from ggaren at apple.com  2007-12-20 11:01 PDT -------
(From update of attachment 18008)
-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.


-- 
Configure bugmail: http://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.



More information about the webkit-unassigned mailing list