Re: svn commit: r900050 - /ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar/TemporalExpressions.java

[Adam Heath-2]

[hidden email] wrote:

> Author: adrianc
> Date: Sun Jan 17 03:13:19 2010
> New Revision: 900050
>
> URL: http://svn.apache.org/viewvc?rev=900050&view=rev
> Log:
> Reorganized code in TemporalExpressions.java. No functional change.
>
> Modified:
>     ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar/TemporalExpressions.java
>
> Modified: ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar/TemporalExpressions.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar/TemporalExpressions.java?rev=900050&r1=900049&r2=900050&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar/TemporalExpressions.java (original)
> +++ ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar/TemporalExpressions.java Sun Jan 17 03:13:19 2010
> @@ -37,224 +37,323 @@
>  public class TemporalExpressions implements Serializable {
>      public static final String module = TemporalExpressions.class.getName();
>      public static final TemporalExpression NullExpression = new Null();
> +    // Expressions are evaluated from largest unit of time to smallest.
> +    // When unit of time is the same, then they are evaluated from
> +    // least ambiguous to most. Frequency should always be first -
> +    // since it is the most specific. Date range should always be last.
> +    // The idea is to evaluate all other expressions, then check to see
> +    // if the result falls within the date range.
> +    public static final int SEQUENCE_DATE_RANGE = 10000;
> +    public static final int SEQUENCE_DAY_IN_MONTH = 400;
> +    public static final int SEQUENCE_DOM_RANGE = 300;
> +    public static final int SEQUENCE_DOW_RANGE = 500;
> +    public static final int SEQUENCE_FREQ = 100;
> +    public static final int SEQUENCE_HOUR_RANGE = 700;
> +    public static final int SEQUENCE_MINUTE_RANGE = 800;
> +    public static final int SEQUENCE_MONTH_RANGE = 200;
> +    public static final int SEQUENCE_TOD_RANGE = 600;

I'd prefer to see this as an enum instead, it offers better type safety.

public enum SEQUENCE {
    DATE_RANGE { public int value() { return 10000; },
    DAY_IN_MONTH { public int value() { return 400; },
    ...
    public abstract int value();
}

If you don't want to have each enum contain a value method, then an
enum map could be used(which is not a real map), but then you'd have
Integer objects, instead of int values.

[Adrian Crum-2]

--- On Sat, 1/16/10, Adam Heath <[hidden email]> wrote:

> From: Adam Heath <[hidden email]>
> Subject: Re: svn commit: r900050 - /ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar/TemporalExpressions.java
> To: [hidden email]
> Date: Saturday, January 16, 2010, 8:20 PM
> [hidden email]
> wrote:
> > Author: adrianc
> > Date: Sun Jan 17 03:13:19 2010
> > New Revision: 900050
> >
> > URL: http://svn.apache.org/viewvc?rev=900050&view=rev
> > Log:
> > Reorganized code in TemporalExpressions.java. No
> functional change.
> >
> > Modified:
> > 
>    ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar/TemporalExpressions.java
> >
> > Modified:
> ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar/TemporalExpressions.java
> > URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar/TemporalExpressions.java?rev=900050&r1=900049&r2=900050&view=diff
> >
> ==============================================================================
> > ---
> ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar/TemporalExpressions.java
> (original)
> > +++
> ofbiz/trunk/framework/service/src/org/ofbiz/service/calendar/TemporalExpressions.java
> Sun Jan 17 03:13:19 2010
> > @@ -37,224 +37,323 @@
> >  public class TemporalExpressions implements
> Serializable {
> >      public static final String module
> = TemporalExpressions.class.getName();
> >      public static final
> TemporalExpression NullExpression = new Null();
> > +    // Expressions are evaluated from
> largest unit of time to smallest.
> > +    // When unit of time is the same, then
> they are evaluated from
> > +    // least ambiguous to most. Frequency
> should always be first -
> > +    // since it is the most specific. Date
> range should always be last.
> > +    // The idea is to evaluate all other
> expressions, then check to see
> > +    // if the result falls within the date
> range.
> > +    public static final int
> SEQUENCE_DATE_RANGE = 10000;
> > +    public static final int
> SEQUENCE_DAY_IN_MONTH = 400;
> > +    public static final int
> SEQUENCE_DOM_RANGE = 300;
> > +    public static final int
> SEQUENCE_DOW_RANGE = 500;
> > +    public static final int SEQUENCE_FREQ =
> 100;
> > +    public static final int
> SEQUENCE_HOUR_RANGE = 700;
> > +    public static final int
> SEQUENCE_MINUTE_RANGE = 800;
> > +    public static final int
> SEQUENCE_MONTH_RANGE = 200;
> > +    public static final int
> SEQUENCE_TOD_RANGE = 600;
>
> I'd prefer to see this as an enum instead, it offers better
> type safety.
>
> public enum SEQUENCE {
>     DATE_RANGE { public int value() { return
> 10000; },
>     DAY_IN_MONTH { public int value() { return
> 400; },
>     ...
>     public abstract int value();
> }
>
> If you don't want to have each enum contain a value method,
> then an
> enum map could be used(which is not a real map), but then
> you'd have
> Integer objects, instead of int values.

That's a good suggestion. I will file it and save it for later. Right now I'm working on Martin Fowler's substitution conundrum.