0
\$\begingroup\$

I need to find fraction of positive, negative and zero values of an array. Here int n is the size of array. I've written the following code:

static void Fraction(int n, int[] a)
{
    double pos = 0, neg = 0, zero = 0;

    for(int i=0, j=n-1; i<j; i++, j--)
    {
        if(a[i] > 0)
            pos++;
        if(a[j] > 0)
            pos++;
        if(a[i] < 0)
            neg++;
        if(a[j] < 0)
            neg++;
        if(a[i] == 0)
            zero++;
        if(a[j] == 0)
            zero++;
    }

    if(n%2!=0)
    {
        int m = n/2;
        if(a[m] > 0) pos++;
        if(a[m] < 0) neg++;
        if(a[m] == 0) zero++;
    }

    Console.WriteLine(pos/n);
    Console.WriteLine(neg/n);
    Console.WriteLine(zero/n);
}

Can we improve this code, specially multiple if conditions?

Note: I found same question on code review but my approach of iterating array is different.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ You can save on the dynamic number of comparisons using the product of two numbers (while the are at least that many - barring overflow: if product below zero, tally one positive, one negative. If product zero, one zero and one of the sign of the sum; otherwise, signs are identical: two of the sign of the first. I tallying needs reduction, just don't count positive numbers: compute their number from n(total) - #zero - #negative. \$\endgroup\$
    – greybeard
    Jul 15, 2017 at 15:44

2 Answers 2

4
\$\begingroup\$

Why are you counting from both ends? That leads to code duplication (since you have every condition twice), degraded performance (because you're accessing two different memory locations, rather than one sequentially, and it is harder for the compiler to optimize), and introduces a bug where you miss counting the middle element if n is odd.

\$\endgroup\$
4
  • \$\begingroup\$ Fixed the bug. I am counting from both ends to reduce iterations. If length of array is 500 this code only do 250 or less loops. Isn't it better than looping size of array when array length is large? \$\endgroup\$
    – SiD
    Jul 15, 2017 at 2:59
  • 2
    \$\begingroup\$ indexing into an array is not a loop. \$\endgroup\$
    – radarbob
    Jul 15, 2017 at 3:53
  • 1
    \$\begingroup\$ +1 for iterating from start to end. Code will be much clearer, without duplications and messing with odd/even array's length. @SiD if you warninig about performance do some benchmarks for your solution and simple array iteration (you can even use foreach). \$\endgroup\$
    – Maxim
    Jul 15, 2017 at 6:00
  • \$\begingroup\$ Guys, thanks.. I just learned "indexing into an array is not a loop".. \$\endgroup\$
    – SiD
    Jul 15, 2017 at 9:38
-2
\$\begingroup\$

I seriously doubt burning from both ends is faster
Use else
Why do you need n if you have the array?
Don't use double for count
Below should be faster than what you have

static void Fraction(int[] a)
{
    int pos = 0, neg = 0, zero = 0;

    for(int i=0; i < a.Length; i++)
    {
        if(a[i] > 0)
        {
            pos++;
        }
        else if(a[i] < 0)
        {
            neg++;
        }
        else  
        {
            zero++;
        }
    }

    Console.WriteLine(pos);
    Console.WriteLine(neg);
    Console.WriteLine(zero);
}
\$\endgroup\$
6
  • \$\begingroup\$ I am not using double to count I used to calculate fraction and n is coming from user input, so just used it instead of re-calculating it. \$\endgroup\$
    – SiD
    Jul 15, 2017 at 9:44
  • \$\begingroup\$ @SiD Uh, double pos = 0, neg = 0, zero = 0; What if that n is wrong? Calculate? \$\endgroup\$
    – paparazzo
    Jul 15, 2017 at 10:09
  • \$\begingroup\$ That's hackerrank puzzle. They are giving me 2 inputs, array and its length. I am sure in this situation n never be wrong because inputs are coming from test cases. \$\endgroup\$
    – SiD
    Jul 15, 2017 at 16:12
  • 1
    \$\begingroup\$ Whatever dude. Assume what you want. It cost no more to read Length over the cost of passing and reading integer. Let it fail if the wrong value passed. You cannot even recognize double. You have sloppy code and discredit a proper answer. Why do you come to code review if you don't intend to improve your code? \$\endgroup\$
    – paparazzo
    Jul 15, 2017 at 16:21
  • \$\begingroup\$ I tried your code and with int pos = 0, neg = 0, zero = 0 it's failing. Please help me understand. I don't meant to offend you. \$\endgroup\$
    – SiD
    Jul 15, 2017 at 17:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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