0
\$\begingroup\$

I am trying to solve the codility problem:

N voracious fish are moving along a river. Calculate how many fish are alive.

Codility Problem

My code get only 50 percent of 100. Could you tell me what is wrong with my code?

You are given two non-empty zero-indexed arrays A and B consisting of N integers. Arrays A and B represent N voracious fish in a river, ordered downstream along the flow of the river.

The fish are numbered from 0 to N − 1. If P and Q are two fish and P < Q, then fish P is initially upstream of fish Q. Initially, each fish has a unique position.

Fish number P is represented by A[P] and B[P]. Array A contains the sizes of the fish. All its elements are unique. Array B contains the directions of the fish. It contains only 0s and/or 1s, where:

0 represents a fish flowing upstream, 1 represents a fish flowing downstream. If two fish move in opposite directions and there are no other (living) fish between them, they will eventually meet each other. Then only one fish can stay alive − the larger fish eats the smaller one. More precisely, we say that two fish P and Q meet each other when P < Q, B[P] = 1 and B[Q] = 0, and there are no living fish between them. After they meet:

If A[P] > A[Q] then P eats Q, and P will still be flowing downstream. If A[Q] > A[P] then Q eats P, and Q will still be flowing upstream. We assume that all the fish are flowing at the same speed. That is, fish moving in the same direction never meet. The goal is to calculate the number of fish that will stay alive.

For example, consider arrays A and B such that:

A[0] = 4    B[0] = 0
A[1] = 3    B[1] = 1
A[2] = 2    B[2] = 0
A[3] = 1    B[3] = 0
A[4] = 5    B[4] = 0

Initially all the fish are alive and all except fish number 1 are moving upstream. Fish number 1 meets fish number 2 and eats it, then it meets fish number 3 and eats it too. Finally, it meets fish number 4 and is eaten by it. The remaining two fish, number 0 and 4, never meet and therefore stay alive.

Write a function:

class Solution { public int solution(int[] A, int[] B); }

that, given two non-empty zero-indexed arrays A and B consisting of N integers, returns the number of fish that will stay alive.

For example, given the arrays shown above, the function should return 2, as explained above.

Assume that:

N is an integer within the range [1..100,000]; each element of array A is an integer within the range [0..1,000,000,000]; each element of array B is an integer that can have one of the following values: 0, 1; the elements of A are all distinct.

Complexity:

Expected worst-case time complexity is O(N); expected worst-case space complexity is O(N), beyond input storage (not counting the storage required for input arguments).

This is my algorithm:

public static int solution(int[] size, int[] direction)
{
    var stack = new Stack<int>();
    const int UPSTREAM = 0;
    const int DOWNSTREAM = 1;
    var updateStack = false;

    for (int i = 0; i < size.Length; i++)
    {
        if (stack.Count <= 0) //initial data
            stack.Push(i);
        else
        {
            var lastFish = stack.Peek();

            if (direction[i] == direction[lastFish] || direction[lastFish] == UPSTREAM && direction[i] == DOWNSTREAM) 
            {
                stack.Push(i);
            }
            else
            {
                while (stack.Count > 0 && size[lastFish] < size[i] && direction[lastFish] == DOWNSTREAM && direction[i] == UPSTREAM)
                {
                    updateStack = true;
                    stack.Pop();
                    if (stack.Count > 0)
                        lastFish = stack.Peek();
                }
                if (updateStack)
                {
                    stack.Push(i);
                    updateStack = false;
                }
            }
        }

    }
    return stack.Count;
}
\$\endgroup\$

closed as off-topic by Mast, 200_success Dec 30 '17 at 22:26

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." – Mast, 200_success
If this question can be reworded to fit the rules in the help center, please edit the question.

  • 2
    \$\begingroup\$ 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. \$\endgroup\$ – Mast Dec 30 '17 at 21:54
  • \$\begingroup\$ I updated the code \$\endgroup\$ – roro2012 Dec 31 '17 at 6:32
  • \$\begingroup\$ If it still only solves 50%, it's not fixed. Please clarify. Did you read the help center? \$\endgroup\$ – Mast Dec 31 '17 at 9:01
  • \$\begingroup\$ Yes, i got only 50% of the total score \$\endgroup\$ – roro2012 Dec 31 '17 at 14:21
  • \$\begingroup\$ That means the code still doesn't work as intended and it's not ready for review. Please take a look at the links provided. \$\endgroup\$ – Mast Dec 31 '17 at 17:53
1
\$\begingroup\$

This is an example of a situation where good naming is your friend. Instead of parameters A and B, and magic numbers 0 and 1, and the rather long-winded expression stack.Peek(), I suggest you start by installing some useful names:

const int UPSTREAM = 0;
const int DOWNSTREAM = 1;

public static int solution(int[] size, int[] direction)
{
    var stack = new Stack<int>();
    var updateStack = false;
    for (int i = 0; i < size.Length; i++)
    {
        if (stack.Count <= 0) //initial data
            stack.Push(i);
        else
        {
            var lastFish = stack.Peek();

            if (direction[i] == direction[lastFish] 
             || direction[i] != direction[lastFish] 
                && direction[lastFish] == UPSTREAM 
                && direction[i] == DOWNSTREAM) 
            {
                stack.Push(i);
            }
            else
            {
                while (stack.Count > 0 
                       && size[lastFish] < size[i] 
                       && direction[lastFish] == DOWNSTREAM 
                       && direction[i] == UPSTREAM)
                {
                    updateStack = true;
                    stack.Pop();
                    lastFish = stack.Peek();
                }
                if (updateStack)
                    stack.Push(i);
            }
        }

    }
    return stack.Count;
}

This shows me there is some confusion on your part. You are allowed to assume that a logical or (||) fails because it's false - there's no need to repeat the test in a false mode:

if (direction[i] == direction[lastFish] 
 || direction[i] != direction[lastFish] && ...

Instead, just assume that failure implies failure, and go on:

if (direction[i] == direction[lastFish] 
  || direction[lastFish] == UPSTREAM 
     && direction[i] == DOWNSTREAM) 

Furthermore, you can assume that since you know the directions are different, and since you know there are only two directions, testing one of them is enough:

if (direction[i] == direction[lastFish] || direction[i] == DOWNSTREAM)

That said, I notice that you initialize updateStack to false outside of your loop and never reset it to false anyplace else. This means that once you've set updateStack = true, it will stay true forever (inside you loop, anyway). That's probably the source of your error, because it should stop being true once you've updated the stack.

\$\endgroup\$
  • \$\begingroup\$ Thanks for help; I updated the code now according to your suggestion but still doesn't work properly \$\endgroup\$ – roro2012 Dec 31 '17 at 5:50
  • \$\begingroup\$ I got only 50% of total score \$\endgroup\$ – roro2012 Dec 31 '17 at 14:21

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