commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Stephen Colebourne" <scolebou...@btopenworld.com>
Subject Re: [lang] ArrayUtils.lastIndex
Date Sun, 10 Oct 2004 18:24:45 GMT
The trouble with this code is that it is religious in nature. By that I mean
its the kind of thing that some prgrammers fervently believe in, and others
do not (a more well known example is curly-brackets at end of line). [lang]
is supposed to stay clear of religious debates where possible.

For me, the constant gets in the way of my understanding of the method. The
javadoc is still going to have to say that it returns -1, so nothing is
actually gained there.Instead, when I read the method, I have to go away to
another part of the file to check that the NOT_FOUND constant (named
INDEX_NOT_FOUND in StringUtils IIRC) is actually set to -1.

Please note that my view pertains only to basic values, like -1 and "". When
the value refers to something more abstract, like an enumerated constant
then a constant makes the code much better and more maintainable. I just
have a place where I draw the line.

As I said before, I'm -0 to the change, as a similar change is already
released in StringUtils. I will however -1 any changes to the javadoc that
cause the doc of each method to not refer to '-1'.

Stephen

----- Original Message -----
From: "Gary Gregory" <ggregory@seagullsoftware.com>
> I don't have a vote, but, if I did,

Well constructed arguments are always welcome, no matter which side of
the debate you fall on ;-)

Just to give a concrete ArrayUtils example of what this would look like
for an existing method:

    public static int indexOf(float[] array, float valueToFind, int
startIndex) {
        if (ArrayUtils.isEmpty(array)) {
            return NOT_FOUND;
        }
        if (startIndex < 0) {
            startIndex = 0;
        }
        for (int i = startIndex; i < array.length; i++) {
            if (valueToFind == array[i]) {
                return i;
            }
        }
        return NOT_FOUND;
    }

which makes it pretty clear IMO that you are returning a *well-known*
value (whatever NOT_FOUND is) for a well-defined purpose; as opposed to
seing "-1" and wondering, why not -2 or -3.

Gary

> -----Original Message-----
> From: Michael McGrady [mailto:mike@michaelmcgrady.com]
> Sent: Saturday, October 09, 2004 14:53
> To: Jakarta Commons Developers List
> Subject: Re: [lang] ArrayUtils.lastIndex
>
> I don't have a vote, but, if I did, I would vote very high in favor of
> all such decisions.  Hard coding is not good for lots of reasons, in
my
> opinion.  If the -1s mean the same thing "all over the place", then
they
> should be identified as to what they mean in all places with the value
> put in one place.  In my opinion, this is one of the more valuable
knee
> jerks we should have.
>
> Michael McGrady
>
> Gary Gregory wrote:
>
> >Speaking of "-1"s. I wonder if we should create a constant for the
"-1
> >means NOT_FOUND" concept. This value is hard coded all over the place
in
> >ArrayUtils. It seems to me like it would make the code more readable.
> >
> >Gary
> >
> >
> >
> >>-----Original Message-----
> >>From: Stephen Colebourne [mailto:scolebourne@btopenworld.com]
> >>Sent: Saturday, October 09, 2004 04:27
> >>To: Jakarta Commons Developers List
> >>Subject: [lang] ArrayUtils.lastIndex
> >>
> >>ArrayUtils.lastIndex
> >>This method is effectively:
> >>  array.length - 1
> >>with null checks
> >>
> >>This method was added in the 1.41 version of ArrayUtils. I want to
> >>
> >>
> >remove
> >
> >
> >>it, as it really doesn't seem to add any value over and above
> >>
> >>
> >getLength().
> >
> >
> >>The original bug request didn't ask for it either:
> >>http://issues.apache.org/bugzilla/show_bug.cgi?id=26594
> >>
> >>I intend to remove the method unless I hear objections.
> >>
> >>Stephen
> >>
> >>
>
>>---------------------------------------------------------------------
> >>To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> >>For additional commands, e-mail: commons-dev-help@jakarta.apache.org
> >>
> >>
> >
> >
> >---------------------------------------------------------------------
> >To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> >For additional commands, e-mail: commons-dev-help@jakarta.apache.org
> >
> >
> >
> >
> >
> >
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Mime
View raw message