HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppModerate

Identifying first negative number in array of doubles

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
numberarraydoublesfirstnegativeidentifying

Problem

I was asked to create a function that checks if an array of doubles has all negative numbers or none. If these 2 (all negative, all positive) are false, check the first negative number, using recursion and only using built-in C++ features.

int first_negative(const double val [] , unsigned array, unsigned short instance_ = 0) //Dont change instance!
{
    //None negative?
    bool none_negative;
    for(int i = 0; i  0) none_negative = true;
        else
        {
            none_negative = false;
            break;
        }
    }
    if(none_negative) return 0;

    //All negative?
    bool negative_instance;
    for(int i = 0; i < array; i++)
    {
        if(val[i] < 0) negative_instance = true;
        else
        {
            negative_instance = false;
            break;
        }
    }

    //Never change instance!
    static short instance = instance_;
    if(array < 1) return 0;
    if(val[instance] < 0) return instance;
    instance++;
    return first_negative(val, array, instance);
}


Can I make it better? Can I remove the instance parameter?

Solution

-
I believe the parameter names val and array are switched around. An array shouldn't be a single variable, and a value shouldn't be an array. Otherwise, the names themselves are okay.

-
It's preferred to index through a C-style array with an std::size_t, especially if the array size is larger than an int. This could be helpful if you'll be working with very large numbers, although the recursion aspect here may render that unlikely. Even if you're not, it's best to develop that habit.

-
This looks a bit misleading:

if(val[instance] < 0) return instance;
instance++;


At first, I thought the instance++ was missing indentation and belonged to the if statement. To make this clearer, you could separate the two lines or, even better, have curly braces for the if statement. The latter also helps with better maintainability.

if (val[instance] < 0)
{
    return instance;
}

instance++;


-
Again, some of the naming is misleading:

if(array < 1) return 0;


From a glance, that just looks incorrect as you cannot check an array like that. Proper naming is very important, even for a small program like this.

Code Snippets

if(val[instance] < 0) return instance;
instance++;
if (val[instance] < 0)
{
    return instance;
}

instance++;
if(array < 1) return 0;

Context

StackExchange Code Review Q#41339, answer score: 14

Revisions (0)

No revisions yet.