commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gary Gregory" <ggreg...@seagullsoftware.com>
Subject RE: [lang] ArrayUtils.lastIndex
Date Mon, 11 Oct 2004 21:13:53 GMT
Hello,

I'll confess to being a member of the OO and refactoring Churches ;-)

I fail to see the comparison between a code formatting example (where to
put curly braces) and the pros and cons of the refactoring being
discussed ('Extract Constant' in Eclipse or 'Replace Magic Number with
Symbolic Constant' in Martin Fowler-speak). 

Yes, some folks have deeply engrained opinions on code formatting;
mercifully, this is not one of these discussions! :-) We are talking
about a well-known refactoring; which is really an even older coding
principle since most programming languages give us the ability to define
and use constants.

Is this a debate where one side is pro-'Replace Magic Number with
Symbolic Constant' and the other against it? It sounds like it, let's
talk about it. 

Note that one always free to ignore constants and use magic numbers in
their code, no matter what the library does internally.

> 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.

A -1 "index" IS abstract! 

-1 represents an idea, not an index. This is a classic case where an API
can return an integer value [0..Integer.MAX_VALUE] which represents one
concept: an index; and under other circumstances, the API returns still
an integer, but this time representing a completely different idea:
there is no index. 

Isn't this a perfect example of a "magic number"?

Gary

> -----Original Message-----
> From: Stephen Colebourne [mailto:scolebourne@btopenworld.com]
> Sent: Sunday, October 10, 2004 11:25
> To: Jakarta Commons Developers List
> Subject: Re: [lang] ArrayUtils.lastIndex
> 
> 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


---------------------------------------------------------------------
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