[threeten-dev] [threeten-develop] Webrev for updates to WeekDefinition

[Roger Riggs]

Thanks for the comments:

The webrev has been updated.

On 12/10/2012 12:22 PM, Stephen Colebourne wrote:
> The Javadoc for dayofWeek() should describe the numering scheme (ie 1 
> to 7, not 0 to 6).
ok
>
> The javadoc for week methods has "Week one(1)" but there should be an 
> extra space "Week one (1)" to match the week zero comment.
ok
>
> I would put a dashed line before the start of the class ;-)
ok
>
> The doIsSupported() method is incomplete for the week fields. It 
> should only return true if the data each week field relies on is 
> available.
ok, DOW only requires ISO DOW, weekOfMonth requires ISO DOW and Month,
weekOfYear requires ISO DOW and year.
>
> Line 410, the range constants should be upper case.
ok
>
> The doRange() method is not implemented. For the week methods it 
> should work out the actual range based on the month/year.
what a pain for little gain, but can be computed from range of the base 
field (month or year).
>
> Not sure how the code handles the case of day-of-week and 
> week-of-month/year with different week definitions. Should merging 
> only happen in resolve() if they have the same week definition? The 
> standard ISO day-of-week should always work, but not sure about mixing 
> week definitions.
Not a problem, the computation for dayOfWeek is based on ISO week and the
WeekDefinition of either weekOfMonth or weekOfYear respectively.

If fields from two different WeekDefinitions are attempted to be resolved
they will conflict in that both will try to define the day of month or year.

>
> I've realised that the toString() on the field should include the 
> toString() of the week definition. This is because otherwise 
> developers will get confused combining day-of-week and week-of-month 
> with different week definitions.
ok
>
> Line 474 comment looks wrong.

updated,

Thanks, Roger
>
> Stephen
>
>
> On 10 December 2012 16:55, Roger Riggs <Roger.Riggs at oracle.com 
> <mailto:Roger.Riggs at oracle.com>> wrote:
>
>     Hi,
>
>     Changes to address Issue #67 Implement Week Rules
>     <https://github.com/ThreeTen/threeten/issues/67> is proposed in
>     this webrev.
>
>     http://cr.openjdk.java.net/~rriggs/weekdef-webrev/
>     <http://cr.openjdk.java.net/%7Erriggs/weekdef-webrev/>
>
>     Comments?
>
>     -- 
>     Thanks, Roger
>
>     Oracle Java Platform Group
>
>     Green Oracle <http://www.oracle.com/commitment> Oracle is
>     committed to developing practices and products that help protect
>     the environment
>
>
>     ------------------------------------------------------------------------------
>     LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
>     Remotely access PCs and mobile devices and provide instant support
>     Improve your efficiency, and focus on delivering more value-add
>     services
>     Discover what IT Professionals Know. Rescue delivers
>     http://p.sf.net/sfu/logmein_12329d2d
>     _______________________________________________
>     threeten-develop mailing list
>     threeten-develop at lists.sourceforge.net
>     <mailto:threeten-develop at lists.sourceforge.net>
>     https://lists.sourceforge.net/lists/listinfo/threeten-develop
>
>
>
>
> ------------------------------------------------------------------------------
> LogMeIn Rescue: Anywhere, Anytime Remote support for IT. Free Trial
> Remotely access PCs and mobile devices and provide instant support
> Improve your efficiency, and focus on delivering more value-add services
> Discover what IT Professionals Know. Rescue delivers
> http://p.sf.net/sfu/logmein_12329d2d
>
>
> _______________________________________________
> threeten-develop mailing list
> threeten-develop at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/threeten-develop

-- 
Thanks, Roger

Oracle Java Platform Group

Green Oracle <http://www.oracle.com/commitment> Oracle is committed to 
developing practices and products that help protect the environment