BotClean challenge

Question

I'm going through the artificial intelligence domain at HackerRank. I'm very new to this domain and I really need a guide on how should I minimize the number of if/else statements in my code.

Problem Statement

The goal of Artificial Intelligence is to create a rational agent (Artificial Intelligence 1.1.4). An agent gets input from the environment through sensors and acts on the environment with actuators. In this challenge, you will program a simple bot to perform the correct actions based on environmental input.

Meet the bot MarkZoid. It's a cleaning bot whose sensor is a head mounted camera and whose actuators are the wheels beneath it. It's used to clean the floor.

The bot here is positioned at the top left corner of a 5*5 grid. Your task is to move the bot to clean all the dirty cells.

Input Format

The first line contains two space separated integers which indicate the current position of the bot. The board is indexed using Matrix Convention 5 lines follow representing the grid. Each cell in the grid is represented by any of the following 3 characters: 'b' (ascii value 98) indicates the bot's current position, 'd' (ascii value 100) indicates a dirty cell and '-' (ascii value 45) indicates a clean cell in the grid.

Note If the bot is on a dirty cell, the cell will still have 'd' on it.

Output Format

The output is the action that is taken by the bot in the current step, and it can be either one of the movements in 4 directions or cleaning up the cell in which it is currently located. The valid output strings are LEFT, RIGHT, UP and DOWN or CLEAN. If the bot ever reaches a dirty cell, output CLEAN to clean the dirty cell. Repeat this process until all the cells on the grid are cleaned.

Sample Input #00

0 0
b---d
-d--d
--dd-
--d--
----d

Sample Output #00

RIGHT

Resultant state

-b--d
-d--d
--dd-
--d--
----d

Sample Input #01

0 1
-b--d
-d--d
--dd-
--d--
----d

Sample Output #01

DOWN

Resultant state

----d
-d--d
--dd-
--d--
----d

Code

The algorithm I'm using is simple: I will first search for wether the bot is in the same row as the cell if it is not it will go up/down as required. Then, I will do the same for column.

#!/usr/bin/ruby
def next_move posr, posc, board
    if board[posr][posc] == 'd'
        puts 'CLEAN'
    elsif board[posr].include? 'd'
        val = posc - board[posr].index('d')
        if val < 0
            puts 'RIGHT'
        else
            puts 'LEFT'
        end
    else
        if posr == 0
            puts 'DOWN'
        else 
            board.each_with_index do |value, index|
                if value.include? 'd'
                    i = index - posr
                    if i < 0
                        puts 'UP'
                    else
                        puts 'DOWN'
                    end
                    break
                end
            end
        end
    end     
end
pos = gets.split.map {|i| i.to_i}
board = Array.new(5)

(0...5).each do |i|
    board[i] = gets.strip
end

puts next_move pos[0], pos[1], board

Answer 1

General tips

• Good, clear variable names are important. While posr and posc might be perfectly clear to you, the more conventional (and therefore more understandable) way to name variables used that way is with row and col/column.
• In function declarations, you should always have parentheses around the arguments, if for no other reason than to make it easier to read.
• Indents in Ruby are typically two spaces. Functionally, it's the same as four, but that's the convention I've seen.

• You should always have a space before and after do and end (or { and } when they're used in that context), so (for example) this:

  pos = gets.split.map {|i| i.to_i}
  

  becomes this:

  pos = gets.split.map { |i| i.to_i }
  

---

Specific stuff

• Your board building bit is a bit confusing -- maybe I just need coffee, but it took me two reads to get how it worked. I'd suggest something like this (using the optional block in Array#new):

  board = Array.new(5) { gets.chomp }
  

  It initializes an array of five elements, and each element is set to the return result of gets (.chomp strips the EOL). Much cleaner, IMO.

• For some reason, you puts the result of next_move, but... next_move doesn't return anything. You're already putting everything in the method. Change all of your puts calls, except the one at the very end, to return, or remove the puts at the very end. Personally, I'd do the former, though I can't articulate why.
• Rather than using i.to_i, I'd suggest using Integer(i). That means it'll fail with an error if you, for example, try to provide ; as one of the coordinates. to_i just fails silently (and returns 0). See the docs for to_i and Integer() for more information.
• As for the meat of your code, though... The algorithm looks good to me. It took me a little while to get it (I really need coffee) but once I did, it was pretty simple.

• You asked about getting rid of your ifs, but I don't really think you need to. Sometimes code just needs a lot of nested conditionals; they're not an anti-pattern, just uncommon. If you really want to, you could use the techniques suggested in the other answer here:

  • Replace things like

    if board[posr][posc] == 'd'
        return 'CLEAN'
    elsif #...
    

    with

    return 'CLEAN' if board[posr][posc] == 'd'
    if #...
    

    (Note that I'm assuming you followed my advice to use return instead of puts; it makes things a lot easier.)

  • Replace everything you can with ternaries, and simplify more! For example, this:

    return 'UP' if index - row < 0
    'DOWN'
    

    becomes this:

    index - row < 0 ? 'UP' : 'DOWN'
    

    Then we can apply the previous tip to the result after this substitution. See the final code for the result.

With all of these suggestions (including the last, since you want it), here's what your code becomes:

#!/usr/bin/ruby
def next_move(row, col, board)
  return 'CLEAN' if board[row][col] == 'd'
  return (col - board[row].index('d') < 0) ? 'RIGHT' : 'LEFT' if board[row].include? 'd'
  return 'DOWN' if row == 0
  board.each_with_index do |value, index|
    return index - row < 0 ? 'UP' : 'DOWN' if value.include? 'd'
  end
end

pos = gets.split.map { |i| Integer(i) }
board = Array.new(5) { gets.chomp }

puts next_move pos[0], pos[1], board

(Note: I decided not to use the implicit return anywhere because it's just simpler to understand that way, with how many places this method returns)

Answer 2

Guard clauses

One way to get away with less if expressions is to use guard clauses: https://sourcemaking.com/refactoring/replace-nested-conditional-with-guard-clauses

For example, you can get rid of your first if by converting it to a guard clause:

if board[posr][posc] == 'd'
  puts 'CLEAN'

Becomes:

puts 'CLEAN' if board[posr][posc] == 'd'

And then your elsif will just be an if. Another thing you can do is to use ternary conditions.

---

Ternary conditions

Ternary conditions have the following form:

condition ? true : false

So you could convert this:

if val < 0
  puts 'RIGHT'
else
  puts 'LEFT'
end

Into this:

puts val < 0 ? 'RIGHT' : 'LEFT'

This gives you a more compact if / else expression.

---

Here is an example:

def next_move(row, col, board)
  return 'CLEAN' if board[row][col] == 'd'

  if board[row].include? 'd'
    (col - board[row].index('d') < 0) ? 'RIGHT' : 'LEFT'
  else    
    row >= board.size ? 'UP' : 'DOWN'
  end
end