You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cayenne.apache.org by Michael Gentry <mg...@masslight.net> on 2009/09/23 16:24:42 UTC

Adding enums?

Should we add more enums to the code?  For example, in Ordering.java,
there are these constants:

public static final boolean ASC = true;
public static final boolean DESC = false;

However, there is no enum to represent the options.  The constructors look like:

public Ordering(String sortPathSpec, boolean ascending) ...

There is nothing to encourage/enforce the usage of the constants, so
you tend to see "true" and "false" being used.  Even in our
documentation:

http://cayenne.apache.org/doc/using-orderings.html

query.addOrdering("artistName", true);
Ordering ordering = new Ordering("artistName", true);

To the new (or even experienced) user, seeing "true" there tends to
have no meaning.  I (and some of the people I've been showing Cayenne
to here) would rather see something like:

Ordering ordering = new Ordering("artistName", OrderingDirection.ASCENDING);

One guy even suggested something like:

Ordering ordering = new AscendingOrdering("artistName");

That is more concise and still explicit, but would obviously require
even more code changes (probably).  Especially when adding the case
insensitive stuff.

Thoughts?

Thanks!

mrg

Re: Adding enums?

Posted by Michael Gentry <mg...@masslight.net>.
That's fine.  The other bugs I wanted to look at were in the modeler,
so no real API changes there.  Of course, I haven't looked at
PersistenceState yet, either ...


On Wed, Sep 30, 2009 at 11:14 AM, Andrus Adamchik
<an...@objectstyle.org> wrote:
>>  For now, I'm using SortOrder, but
>> I'm sure that can change (be refactored) before 3.0 goes final if a
>> better name is desired.
>
> That has to be "before 3.0 goes Beta", as our beta promise is no API
> changes, unless absolutely required to fix a bug.
>
> Andrus

Re: Adding enums?

Posted by Andrus Adamchik <an...@objectstyle.org>.
On Sep 30, 2009, at 6:13 PM, Michael Gentry wrote:

> I've got a first cut of this ready to check-in after I create a ticket
> (which I'll have to do at home).

Great.

>  For now, I'm using SortOrder, but
> I'm sure that can change (be refactored) before 3.0 goes final if a
> better name is desired.

That has to be "before 3.0 goes Beta", as our beta promise is no API  
changes, unless absolutely required to fix a bug.

Andrus

Re: Adding enums?

Posted by Michael Gentry <mg...@masslight.net>.
I've got a first cut of this ready to check-in after I create a ticket
(which I'll have to do at home).  For now, I'm using SortOrder, but
I'm sure that can change (be refactored) before 3.0 goes final if a
better name is desired.

mrg

Re: Adding enums?

Posted by Matt Kerr <ma...@centralparksoftware.com>.
thanks for the post mike.  good stuff.

> public enum Direction/SortOrder {
>    ASCENDING, ASCENDING_INSENSITIVE, DESCENDING, DESCENDING_INSENSITIVE
> }

just a thought
are these actually something like 'collation functions'
ie
http://en.wikipedia.org/wiki/Collation#Collation_systems
http://www.merriam-webster.com/dictionary/collating

maybe something like :-?
~~
public Ordering(String pathSpec, Collator collator) {...}
~~




On Fri, Sep 25, 2009 at 10:27 AM, Michael Gentry <mg...@masslight.net> wrote:
> I've started looking into this.  How about calling the sorting
> direction enum Direction or SortOrder instead of Order?  (I can see
> people using an ORM having an Order class already, which might be a
> bit confusing ... sorting your Orders by an Order ...)  With the
> change, you'd end up with a constructor something like:
>
> public Ordering(String sortPathSpec, Direction/SortOrder sortingDirection) {...}
>
> I think we should fix the case sensitive flag while we are at it.
> Thoughts on the name for that one?  Here is the constructor ...
>
> public Ordering(String sortPathSpec, Direction/SortOrder
> sortingDirection, boolean caseInsensitive) {...}
>
> One option is to make the Direction/SortOrder enum look something like:
>
> public enum Direction/SortOrder {
>    ASCENDING, ASCENDING_INSENSITIVE, DESCENDING, DESCENDING_INSENSITIVE
> }
>
> Then you don't need the third parameter.
>
> Thanks,
>
> mrg
>

Re: Adding enums?

Posted by Andrus Adamchik <an...@objectstyle.org>.
On Sep 25, 2009, at 5:27 PM, Michael Gentry wrote:

> I've started looking into this.  How about calling the sorting
> direction enum Direction or SortOrder instead of Order?  (I can see
> people using an ORM having an Order class already, which might be a
> bit confusing ... sorting your Orders by an Order ...)  With the
> change, you'd end up with a constructor something like:
>
> public Ordering(String sortPathSpec, Direction/SortOrder  
> sortingDirection) {...}

I guess any of the above can potentially be an entity name, but I am  
fine with either of the 3.


> I think we should fix the case sensitive flag while we are at it.
> Thoughts on the name for that one?  Here is the constructor ...
>
> public Ordering(String sortPathSpec, Direction/SortOrder
> sortingDirection, boolean caseInsensitive) {...}
>
> One option is to make the Direction/SortOrder enum look something  
> like:
>
> public enum Direction/SortOrder {
>    ASCENDING, ASCENDING_INSENSITIVE, DESCENDING,  
> DESCENDING_INSENSITIVE
> }
>
> Then you don't need the third parameter.
>

Good idea.

Andrus


Re: Adding enums?

Posted by Michael Gentry <mg...@masslight.net>.
I've started looking into this.  How about calling the sorting
direction enum Direction or SortOrder instead of Order?  (I can see
people using an ORM having an Order class already, which might be a
bit confusing ... sorting your Orders by an Order ...)  With the
change, you'd end up with a constructor something like:

public Ordering(String sortPathSpec, Direction/SortOrder sortingDirection) {...}

I think we should fix the case sensitive flag while we are at it.
Thoughts on the name for that one?  Here is the constructor ...

public Ordering(String sortPathSpec, Direction/SortOrder
sortingDirection, boolean caseInsensitive) {...}

One option is to make the Direction/SortOrder enum look something like:

public enum Direction/SortOrder {
    ASCENDING, ASCENDING_INSENSITIVE, DESCENDING, DESCENDING_INSENSITIVE
}

Then you don't need the third parameter.

Thanks,

mrg

Re: Adding enums?

Posted by Michael Gentry <mg...@masslight.net>.
I was favoring #1.  #2 was just a thought to keep it similar to what
already exists in the Ordering class, but didn't seem as good to me.


On Wed, Sep 23, 2009 at 11:13 AM, Andrus Adamchik
<an...@objectstyle.org> wrote:
>
> On Sep 23, 2009, at 6:09 PM, Michael Gentry wrote:
>
>> #1: Call the enum class Order.  I think Order.ASCENDING would be
>> readable and intuitive.
>
> +1
>
>>
>> #2: Keep it something more descriptive like OrderingDirection, but in
>> the Ordering class have a "public static final OrderingDirection
>> ASCENDING = OrderingDirection.ASCENDING;".  Then you could reference
>> Ordering.ASCENDING (kind of like Ordering.ASC).  This seems awkward,
>> though, but makes it similar to what is already there.
>
> Don't understand this one. Why the new Order enum is not sufficient?
>
> Andrus
>
>

Re: Adding enums?

Posted by Andrus Adamchik <an...@objectstyle.org>.
On Sep 23, 2009, at 6:09 PM, Michael Gentry wrote:

> #1: Call the enum class Order.  I think Order.ASCENDING would be
> readable and intuitive.

+1

>
> #2: Keep it something more descriptive like OrderingDirection, but in
> the Ordering class have a "public static final OrderingDirection
> ASCENDING = OrderingDirection.ASCENDING;".  Then you could reference
> Ordering.ASCENDING (kind of like Ordering.ASC).  This seems awkward,
> though, but makes it similar to what is already there.

Don't understand this one. Why the new Order enum is not sufficient?

Andrus


Re: Adding enums?

Posted by Michael Gentry <mg...@masslight.net>.
Two other ideas ...

#1: Call the enum class Order.  I think Order.ASCENDING would be
readable and intuitive.

#2: Keep it something more descriptive like OrderingDirection, but in
the Ordering class have a "public static final OrderingDirection
ASCENDING = OrderingDirection.ASCENDING;".  Then you could reference
Ordering.ASCENDING (kind of like Ordering.ASC).  This seems awkward,
though, but makes it similar to what is already there.

mrg


On Wed, Sep 23, 2009 at 10:55 AM, Andrus Adamchik
<an...@objectstyle.org> wrote:
>
> Yeah, I never use it at all.
>
>
> On Sep 23, 2009, at 5:44 PM, Michael Gentry wrote:
>
>> BTW, I'm a little iffy on the static import stuff.  It seems less
>> explicit and somewhat cheating to me, but I'm just tossing the idea
>> out for feedback.
>>
>>
>> On Wed, Sep 23, 2009 at 10:40 AM, Michael Gentry <mg...@masslight.net>
>> wrote:
>>>
>>> If wanting to keep things shorter, couldn't you do a static import?
>>> I'm a little rusty on the syntax, but wouldn't it be something like:
>>>
>>> import static ...query.QueryDirection.*;
>>>
>>> Ordering ordering = new Ordering("artistName", ASCENDING);
>>>
>>> Hmm, or maybe that only works for static constants and not enums.  I'd
>>> have to test that.
>>>
>>> mrg
>>>
>>>
>>> On Wed, Sep 23, 2009 at 10:34 AM, Andrus Adamchik
>>> <an...@objectstyle.org> wrote:
>>>>
>>>> +1
>>>>
>>>> If we can do clean gradual deprecation of the old API. Also I like us
>>>> using
>>>> ASCENDING instead of ASC, as you proposed, but wonder if we can call
>>>> OrderingDirection something shorter, just to keep things tight.
>>>>
>>>> Another candidate for such refactoring is PersistenceState. Maybe create
>>>> a
>>>> "State" enum or "PersistentState"?
>>>>
>>>> Andrus
>>>>
>>>> On Sep 23, 2009, at 5:24 PM, Michael Gentry wrote:
>>>>
>>>>> Should we add more enums to the code?  For example, in Ordering.java,
>>>>> there are these constants:
>>>>>
>>>>> public static final boolean ASC = true;
>>>>> public static final boolean DESC = false;
>>>>>
>>>>> However, there is no enum to represent the options.  The constructors
>>>>> look
>>>>> like:
>>>>>
>>>>> public Ordering(String sortPathSpec, boolean ascending) ...
>>>>>
>>>>> There is nothing to encourage/enforce the usage of the constants, so
>>>>> you tend to see "true" and "false" being used.  Even in our
>>>>> documentation:
>>>>>
>>>>> http://cayenne.apache.org/doc/using-orderings.html
>>>>>
>>>>> query.addOrdering("artistName", true);
>>>>> Ordering ordering = new Ordering("artistName", true);
>>>>>
>>>>> To the new (or even experienced) user, seeing "true" there tends to
>>>>> have no meaning.  I (and some of the people I've been showing Cayenne
>>>>> to here) would rather see something like:
>>>>>
>>>>> Ordering ordering = new Ordering("artistName",
>>>>> OrderingDirection.ASCENDING);
>>>>>
>>>>> One guy even suggested something like:
>>>>>
>>>>> Ordering ordering = new AscendingOrdering("artistName");
>>>>>
>>>>> That is more concise and still explicit, but would obviously require
>>>>> even more code changes (probably).  Especially when adding the case
>>>>> insensitive stuff.
>>>>>
>>>>> Thoughts?
>>>>>
>>>>> Thanks!
>>>>>
>>>>> mrg
>>>>>
>>>>
>>>>
>>>
>>
>
>

Re: Adding enums?

Posted by Andrus Adamchik <an...@objectstyle.org>.
Yeah, I never use it at all.


On Sep 23, 2009, at 5:44 PM, Michael Gentry wrote:

> BTW, I'm a little iffy on the static import stuff.  It seems less
> explicit and somewhat cheating to me, but I'm just tossing the idea
> out for feedback.
>
>
> On Wed, Sep 23, 2009 at 10:40 AM, Michael Gentry <mgentry@masslight.net 
> > wrote:
>> If wanting to keep things shorter, couldn't you do a static import?
>> I'm a little rusty on the syntax, but wouldn't it be something like:
>>
>> import static ...query.QueryDirection.*;
>>
>> Ordering ordering = new Ordering("artistName", ASCENDING);
>>
>> Hmm, or maybe that only works for static constants and not enums.   
>> I'd
>> have to test that.
>>
>> mrg
>>
>>
>> On Wed, Sep 23, 2009 at 10:34 AM, Andrus Adamchik
>> <an...@objectstyle.org> wrote:
>>> +1
>>>
>>> If we can do clean gradual deprecation of the old API. Also I like  
>>> us using
>>> ASCENDING instead of ASC, as you proposed, but wonder if we can call
>>> OrderingDirection something shorter, just to keep things tight.
>>>
>>> Another candidate for such refactoring is PersistenceState. Maybe  
>>> create a
>>> "State" enum or "PersistentState"?
>>>
>>> Andrus
>>>
>>> On Sep 23, 2009, at 5:24 PM, Michael Gentry wrote:
>>>
>>>> Should we add more enums to the code?  For example, in  
>>>> Ordering.java,
>>>> there are these constants:
>>>>
>>>> public static final boolean ASC = true;
>>>> public static final boolean DESC = false;
>>>>
>>>> However, there is no enum to represent the options.  The  
>>>> constructors look
>>>> like:
>>>>
>>>> public Ordering(String sortPathSpec, boolean ascending) ...
>>>>
>>>> There is nothing to encourage/enforce the usage of the constants,  
>>>> so
>>>> you tend to see "true" and "false" being used.  Even in our
>>>> documentation:
>>>>
>>>> http://cayenne.apache.org/doc/using-orderings.html
>>>>
>>>> query.addOrdering("artistName", true);
>>>> Ordering ordering = new Ordering("artistName", true);
>>>>
>>>> To the new (or even experienced) user, seeing "true" there tends to
>>>> have no meaning.  I (and some of the people I've been showing  
>>>> Cayenne
>>>> to here) would rather see something like:
>>>>
>>>> Ordering ordering = new Ordering("artistName",
>>>> OrderingDirection.ASCENDING);
>>>>
>>>> One guy even suggested something like:
>>>>
>>>> Ordering ordering = new AscendingOrdering("artistName");
>>>>
>>>> That is more concise and still explicit, but would obviously  
>>>> require
>>>> even more code changes (probably).  Especially when adding the case
>>>> insensitive stuff.
>>>>
>>>> Thoughts?
>>>>
>>>> Thanks!
>>>>
>>>> mrg
>>>>
>>>
>>>
>>
>


Re: Adding enums?

Posted by Michael Gentry <mg...@masslight.net>.
BTW, I'm a little iffy on the static import stuff.  It seems less
explicit and somewhat cheating to me, but I'm just tossing the idea
out for feedback.


On Wed, Sep 23, 2009 at 10:40 AM, Michael Gentry <mg...@masslight.net> wrote:
> If wanting to keep things shorter, couldn't you do a static import?
> I'm a little rusty on the syntax, but wouldn't it be something like:
>
> import static ...query.QueryDirection.*;
>
> Ordering ordering = new Ordering("artistName", ASCENDING);
>
> Hmm, or maybe that only works for static constants and not enums.  I'd
> have to test that.
>
> mrg
>
>
> On Wed, Sep 23, 2009 at 10:34 AM, Andrus Adamchik
> <an...@objectstyle.org> wrote:
>> +1
>>
>> If we can do clean gradual deprecation of the old API. Also I like us using
>> ASCENDING instead of ASC, as you proposed, but wonder if we can call
>> OrderingDirection something shorter, just to keep things tight.
>>
>> Another candidate for such refactoring is PersistenceState. Maybe create a
>> "State" enum or "PersistentState"?
>>
>> Andrus
>>
>> On Sep 23, 2009, at 5:24 PM, Michael Gentry wrote:
>>
>>> Should we add more enums to the code?  For example, in Ordering.java,
>>> there are these constants:
>>>
>>> public static final boolean ASC = true;
>>> public static final boolean DESC = false;
>>>
>>> However, there is no enum to represent the options.  The constructors look
>>> like:
>>>
>>> public Ordering(String sortPathSpec, boolean ascending) ...
>>>
>>> There is nothing to encourage/enforce the usage of the constants, so
>>> you tend to see "true" and "false" being used.  Even in our
>>> documentation:
>>>
>>> http://cayenne.apache.org/doc/using-orderings.html
>>>
>>> query.addOrdering("artistName", true);
>>> Ordering ordering = new Ordering("artistName", true);
>>>
>>> To the new (or even experienced) user, seeing "true" there tends to
>>> have no meaning.  I (and some of the people I've been showing Cayenne
>>> to here) would rather see something like:
>>>
>>> Ordering ordering = new Ordering("artistName",
>>> OrderingDirection.ASCENDING);
>>>
>>> One guy even suggested something like:
>>>
>>> Ordering ordering = new AscendingOrdering("artistName");
>>>
>>> That is more concise and still explicit, but would obviously require
>>> even more code changes (probably).  Especially when adding the case
>>> insensitive stuff.
>>>
>>> Thoughts?
>>>
>>> Thanks!
>>>
>>> mrg
>>>
>>
>>
>

Re: Adding enums?

Posted by Michael Gentry <mg...@masslight.net>.
If wanting to keep things shorter, couldn't you do a static import?
I'm a little rusty on the syntax, but wouldn't it be something like:

import static ...query.QueryDirection.*;

Ordering ordering = new Ordering("artistName", ASCENDING);

Hmm, or maybe that only works for static constants and not enums.  I'd
have to test that.

mrg


On Wed, Sep 23, 2009 at 10:34 AM, Andrus Adamchik
<an...@objectstyle.org> wrote:
> +1
>
> If we can do clean gradual deprecation of the old API. Also I like us using
> ASCENDING instead of ASC, as you proposed, but wonder if we can call
> OrderingDirection something shorter, just to keep things tight.
>
> Another candidate for such refactoring is PersistenceState. Maybe create a
> "State" enum or "PersistentState"?
>
> Andrus
>
> On Sep 23, 2009, at 5:24 PM, Michael Gentry wrote:
>
>> Should we add more enums to the code?  For example, in Ordering.java,
>> there are these constants:
>>
>> public static final boolean ASC = true;
>> public static final boolean DESC = false;
>>
>> However, there is no enum to represent the options.  The constructors look
>> like:
>>
>> public Ordering(String sortPathSpec, boolean ascending) ...
>>
>> There is nothing to encourage/enforce the usage of the constants, so
>> you tend to see "true" and "false" being used.  Even in our
>> documentation:
>>
>> http://cayenne.apache.org/doc/using-orderings.html
>>
>> query.addOrdering("artistName", true);
>> Ordering ordering = new Ordering("artistName", true);
>>
>> To the new (or even experienced) user, seeing "true" there tends to
>> have no meaning.  I (and some of the people I've been showing Cayenne
>> to here) would rather see something like:
>>
>> Ordering ordering = new Ordering("artistName",
>> OrderingDirection.ASCENDING);
>>
>> One guy even suggested something like:
>>
>> Ordering ordering = new AscendingOrdering("artistName");
>>
>> That is more concise and still explicit, but would obviously require
>> even more code changes (probably).  Especially when adding the case
>> insensitive stuff.
>>
>> Thoughts?
>>
>> Thanks!
>>
>> mrg
>>
>
>

Re: Adding enums?

Posted by Andrus Adamchik <an...@objectstyle.org>.
+1

If we can do clean gradual deprecation of the old API. Also I like us  
using ASCENDING instead of ASC, as you proposed, but wonder if we can  
call OrderingDirection something shorter, just to keep things tight.

Another candidate for such refactoring is PersistenceState. Maybe  
create a "State" enum or "PersistentState"?

Andrus

On Sep 23, 2009, at 5:24 PM, Michael Gentry wrote:

> Should we add more enums to the code?  For example, in Ordering.java,
> there are these constants:
>
> public static final boolean ASC = true;
> public static final boolean DESC = false;
>
> However, there is no enum to represent the options.  The  
> constructors look like:
>
> public Ordering(String sortPathSpec, boolean ascending) ...
>
> There is nothing to encourage/enforce the usage of the constants, so
> you tend to see "true" and "false" being used.  Even in our
> documentation:
>
> http://cayenne.apache.org/doc/using-orderings.html
>
> query.addOrdering("artistName", true);
> Ordering ordering = new Ordering("artistName", true);
>
> To the new (or even experienced) user, seeing "true" there tends to
> have no meaning.  I (and some of the people I've been showing Cayenne
> to here) would rather see something like:
>
> Ordering ordering = new Ordering("artistName",  
> OrderingDirection.ASCENDING);
>
> One guy even suggested something like:
>
> Ordering ordering = new AscendingOrdering("artistName");
>
> That is more concise and still explicit, but would obviously require
> even more code changes (probably).  Especially when adding the case
> insensitive stuff.
>
> Thoughts?
>
> Thanks!
>
> mrg
>