[OCLUG-devel] ArrayMaxValue() - how could I improve it?

[Tim Thelin]

James Colannino wrote:

> I know that one limitation I'd like to do away with is the fact that 
> instead of just parsing through the array until the user's input is 
> finished, it's going to go all the way up to element 20, even if the 
> user only enters two or three numbers.

There are two ways to deal with this, either have a value in the array 
which means "i'm an endpoint so don't go past me" or you store the 
number of valid entries somewhere.
For instance a C string is the first case, you know there are no more 
elements when you hit null '\0'
An example of the second case would be having a data structure that 
contained your array and had a size field.... so something like
struct mydata
{
    int array[ 20 ];
    int numValidEntries;
}

>   Also, how does my coding style look?  Are my comments too verbose?  
> Does anything about the way I make use of whitespace make things more 
> confusing?

Style is really subjective, just make sure you and anyone you work with 
agree with the style chosen.
One of the biggest issues with software is maintaining it once its 
written, so comments are a Good Thing (tm).  However don't comment the 
obvious or its harder to follow whats going on (visual clutter).  So 
comment what people might not understand immediatly.  This is learned 
with practice.
Also, make use of the C++ "//" style comments as (at least for me) its 
less visual clutter than "/* ... */" everywhere.

Whitespace usage is also subjective, so use what makes the code more 
readble to you.  I prefer to use 4 spaces for my indents to make them 
stand out and I make moderate use of blank lines so that I can make 
groups of statements that act together.  All this is really preference 
though.  Too many blank lines, etc, and the code becomes harder to read 
because your eyes must have to move so much.


Btw a "for" loop would probably be better than the "while" (did the book 
cover "for" loops yet?)
so something like:

int position = 0;
int maxValue = 0

for( position = 0; position < 20; position++ )
{
     number = array[ position ];

     if( number > maxValue )
           maxValue = number
}

return maxValue;


Btw, potential issues in the code that can cause bugs:
* I don't see "position" incremented anywhere in your function... so is 
it really looping through the array correctly?
* MaxValue starts at 0, so if your array was all negative numbers ( i.e. 
{ -1, -100, -60 } ), it would return 0 instead of the largest negative 
(i.e. -1)


Thanks,
Tim Thelin