patterncppModerate
Identifying first negative number in array of doubles
Viewed 0 times
numberarraydoublesfirstnegativeidentifying
Problem
I was asked to create a function that checks if an array of
Can I make it better? Can I remove the
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
-
It's preferred to index through a C-style array with an
-
This looks a bit misleading:
At first, I thought the
-
Again, some of the naming is misleading:
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.
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.