Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have a function that produce an array with values that are Gaussian(normal) distribute. If you plot the values on a graph, it would look something like this:

Gussian

Is there any way to improve on the code?

private static IComparable[] NonUniformDistributionsGaussian(int startNumber,int arraySize)
    {
        IComparable [] arr = new IComparable[arraySize];
        for (var i = 0; i < arraySize; i++)
        {

            if (i <= (int)((1.00 / 3.00) * arraySize))
            {
                startNumber = startNumber + 1;
                arr[i] =startNumber;
            }

            if (i >= (int)((1.00 / 3.00) * arraySize) && i <= ((2.00 / 3.00) * (double)arraySize))
            {
                 startNumber = startNumber  + 2; 
                 arr[i] = startNumber;
            }

            if ( i > ((2.00 / 3.00) * arraySize))
            {
                 startNumber = startNumber  - 2; 
                 arr[i] = startNumber;
            }
        }
        return arr;
    }
share|improve this question

put on hold as off-topic by 200_success 2 days ago

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions containing broken code or asking for advice about code not yet written are off-topic, as the code is not ready for review. After the question has been edited to contain working code, we will consider reopening it." – 200_success
If this question can be reworded to fit the rules in the help center, please edit the question.

    
The code doesn't do what you claim. –  200_success 2 days ago

2 Answers 2

As the conditions of the if statements only depending on some fixed numbers and the arraysize input parameter these values can be precalculated.

int oneThirdArraySize = (int)((1.00 / 3.00) * arraySize);

and then used in the loop like

for (var i = 0; i < arraySize; i++)
{

    if (i <= oneThirdArraySize)
    {
        startNumber = startNumber + 1;
        arr[i] = startNumber;
    }
    // and so on
}

By precalculation of the right side values of the if conditions outside of the loop you can speed this up because right now you do these calculation up to two times for every iteration.

But we can do better because you are using some magic numbers here which we can hide behind some meaningful const variables.

private static const double oneThird = 1d / 3d;
private static const double twoThird = 2d / 3d;  

In this way the calculations need to be changed to

int oneThirdArraySize = (int)(oneThird * arraySize);
share|improve this answer
    
I agree this is an excellent point. Some more details would be very good –  donquixto Dac 2 days ago

I don't understand the logic or how it generates a Gaussian distribution.

Assuming the logic itself is correct, you could simplify the code.

Key points:

  • assignment statement identical in all if branches, so pull it out of the conditional logic (DRY principle)
  • No reason to do floating-point arithmetic - array indexes have to be integers so it's not clear what you're accomplishing
  • use if/else to avoid repeating the inverse of the first if branch (DRY principle again)
  • since there are only 3 mutually exclusive possibilities of the array index (first third, second third, final third) you don't even need to specify the final condition explicitly.

Improved version:

private static IComparable[] NonUniformDistributionsGaussian(int startNumber, int arraySize)
{
    IComparable [] arr = new IComparable[arraySize];
    for (var i = 0; i < arraySize; i++)
    {

        if (i <= arraySize / 3)
        {
            startNumber = startNumber + 1;
        }
        else if (i <= 2*arraySize/3)
        {
             startNumber = startNumber  + 2; 
        }
        else
        {
             startNumber = startNumber  - 2; 
        }
        arr[i] = startNumber;
    }
    return arr;
}
share|improve this answer
    
why i say it is a Gaussian distribution function is that the values in the array when plot on graph will look similar to the graph in my question. –  donquixto Dac 2 days ago

Not the answer you're looking for? Browse other questions tagged or ask your own question.