Codility voracious fish are moving along a river, solution in C#

Question

This is the task's description. My code gets 100% task score. Is it good code yet? (I think I'm getting a lot better, since I started solving this codility's exercises and posting here, but I always get important things to improve -which help me A LOT!- from reviewers)

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

---

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).

using System;
using System.Collections.Generic;
using System.Linq;

class Solution
{
    public int solution(int[] A, int[] B)
    {
        const int UpstreamFloater = 0;
        const int DownstreamFloater = 1;

        Stack<int> downstreamersStack = new Stack<int>();

        int topStackDownstreamer;

        int aliveFishesCounter = 0;

        for (int fish = 0; fish < B.Length; fish++)
        {
            if (downstreamersStack.Count == 0 && B[fish] == UpstreamFloater)
            {
                aliveFishesCounter++;
            }
            else if (B[fish] == DownstreamFloater)
            {
                downstreamersStack.Push(fish);
            }
            else if (B[fish] == UpstreamFloater)
            {
                topStackDownstreamer = downstreamersStack.Peek();
                while (downstreamersStack.Count > 0 && A[fish] > A[topStackDownstreamer])
                {
                    downstreamersStack.Pop();
                    if (downstreamersStack.Count > 0)
                    {
                        topStackDownstreamer = downstreamersStack.Peek();
                    }
                }
                if (downstreamersStack.Count == 0)
                {
                    aliveFishesCounter++;
                }
            }
        }
        aliveFishesCounter += downstreamersStack.Count;
        return aliveFishesCounter;
    }
}

Answer 1

Your solution is good. There are a couple bits you could change, sure. Some of them boil down to personal preferences.

Give the arrays more useful names:

public int solution(int[] size, int[] direction)

Declare topStackDownstreamer when you need it. Keeping the scope of a variable minimal gives you less things to remember. What is topStackDownstreamer? How is it used? What was it's value when we assign a new one? Should we do something with the old value first?

But you might as well not need topStackDownstreamer at all.

As you use if and two else if your code looks like it is missing something - the last else. I'd recommend using one if and else for clarity. You could also restructure the stack looping a little bit and check for stacksize=0 only once:

        if (direction[fish] == DownstreamFloater)
        {
            downstreamersStack.Push(fish);
        }
        else // must be UpstreamFloater
        {
            while (downstreamersStack.Count > 0)
            {
                if(size[fish] > size[downstreamersStack.Peek()])
                {
                    downstreamersStack.Pop();
                }
                else
                {
                    break;
                }
            }

            if (downstreamersStack.Count == 0)
            {
                aliveFishesCounter++;
            }
        }

Another little bit - you could return without storing the added variables:

return aliveFishesCounter + downstreamersStack.Count;

The last bit - maybe you could use shorter identifiers. e.g. direction[fish] == DownstreamFloater would look better if it was direction[fish] == Downstream. Another example aliveFishesCounter could be survivors.