4
\$\begingroup\$

Today I have attempted a Codility exercise "Fish" (https://app.codility.com/programmers/lessons/7-stacks_and_queues/fish/), and I have to say I found it to be: 1) Not necessarily "easy" as implied by the page, AND 2) Quite satisfying to figure out in the end. Below the text of the challenge:

You are given two non-empty 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.

My solution is a bit playful, and I was wondering if anyone could have a look and let me know if this is a correct way of solving this problem. All comments will be appreciated.

import java.util.ArrayDeque;

// you can write to stdout for debugging purposes, e.g.
// System.out.println("this is a debug message");

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

        ArrayDeque<Fish> safetyPool = new ArrayDeque<Fish>();
        ArrayDeque<Fish> activePool = new ArrayDeque<Fish>();

        for (int i = 0; i < A.length; i++) {
            activePool.offer(Fish.cFish(A[i], B[i]));
        }

        while(!activePool.isEmpty()) {
            // System.out.println(safetyPool + " " + activePool);
            Fish safeFish = safetyPool.peekLast();
            Fish swimmingFish = activePool.peekFirst();

            if (safeFish != null) {
                if (safeFish.dir == 1) {
                    if (safeFish.dir != swimmingFish.dir) {
                        if (safeFish.att > swimmingFish.att) {
                            activePool.removeFirst();
                        } else {
                            safetyPool.removeLast();
                        }
                    } else {
                        safetyPool.offer(activePool.pollFirst());
                    }
                } else {
                    safetyPool.offer(activePool.pollFirst());
                }
            } else {
                safetyPool.offer(activePool.pollFirst());
            }
        }
        return safetyPool.size();
    }
}

class Fish {
    public int att;
    public int dir;

    public Fish(int att, int dir) {
        this.att = att;
        this.dir = dir;
    }

    public static Fish cFish(int att, int dir) {
        return new Fish(att, dir);
    }

    @Override
    public String toString() {
        return String.format("Fish(att: %d, dir: %d)", att, dir);
    }
}
```
\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review, I edited your post adding the text of the challenge. \$\endgroup\$ – dariosicily Feb 25 '20 at 15:19
3
\$\begingroup\$

I have some suggestions.

  1. When declaring variable that has inheritance, uses the interface in the left declaration part; this will make the code easier to refactor.

Before

ArrayDeque<Fish> safetyPool = new ArrayDeque<Fish>();
ArrayDeque<Fish> activePool = new ArrayDeque<Fish>();

After

Deque<Fish> safetyPool = new ArrayDeque<Fish>();
Deque<Fish> activePool = new ArrayDeque<Fish>();
  1. When using the diamond operators with variables, declared the type in the left side, the right one will become optional; since it's declared in the variable side.
Deque<Fish> safetyPool = new ArrayDeque<>();
Deque<Fish> activePool = new ArrayDeque<>();
  1. To remove the Arrow Code in the while, I suggest that you read the blog post made by Jeff Atwood Flattening Arrow Code. In my opinion, it's the best way to get rid this kind of code.

The logic contained in the while can be refactored.

while (!activePool.isEmpty()) {
   Fish safeFish = safetyPool.peekLast();
   Fish swimmingFish = activePool.peekFirst();

   if (safeFish == null || safeFish.dir != 1 || safeFish.dir == swimmingFish.dir) {
      safetyPool.offer(activePool.pollFirst());
      continue;
   }

   if (safeFish.att > swimmingFish.att) {
      activePool.removeFirst();
   } else {
      safetyPool.removeLast();
   }
}
\$\endgroup\$
  • \$\begingroup\$ Thanks Doi9t, I really appreciate your comments. I am still new to Java, and have seen the approach from paragraphs 1&2 of your answer in other spots, but could never figure out why it was being done this way. Now I know, that this is to allow us an easier access to polymorphic features downstream from the declarations. Paragraph 3 is pure gold as well - I am one of these programmers, who sharpens the arrow head quite a lot at times. The article you linked really helps with change of my habits. A+ answer! :) \$\endgroup\$ – Greem666 Feb 26 '20 at 2:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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