I have already flunked the test with this code so don't feel bad for cheating any employer. I didn't get any feedback though just wondering what could have gone wrong.You don't need to get into the functionality, just the overall design. What are short-coming of this code which disqualifies it from being production quality.Toy Robot SimulatorDescriptionThe application is a simulation of a toy robot moving on a square tabletop,   of dimensions 5 units x 5 units.There are no other obstructions on the table surface.The robot is free to roam around the surface of the table, but must be   prevented from falling to destruction. Any movement that would  result in the   robot falling from the table must be prevented,  however further valid   movement commands must still be allowed.Create an application that can read in commands of the following form:PLACE X,Y,FMOVELEFTRIGHTREPORTPLACE will put the toy robot on the table in position X,Y and facing NORTH,   SOUTH, EAST or WEST.The origin (0,0) can be considered to be the SOUTH WEST most corner.The first valid command to the robot is a PLACE command, after that, any   sequence of commands may be issued, in any order, including  another PLACE   command. The application should discard all commands  in the sequence until   a valid PLACE command has been executed.MOVE will move the toy robot one unit forward in the direction it is   currently facing.LEFT and RIGHT will rotate the robot 90 degrees in the specified direction   without changing the position of the robot.REPORT will announce the X,Y and F of the robot. This can be in any form,   but standard output is sufficient.A robot that is not on the table can choose the ignore the MOVE, LEFT, RIGHT   and REPORT commands.Input can be from a file, or from standard input, as the developer chooses.Provide test data to exercise the application.ConstraintsThe toy robot must not fall off the table during movement. This also   includes the initial placement of the toy robot.Any move that would cause the robot to fall must be ignored.Example Input and OutputExample aPLACE 0,0,NORTHMOVEREPORTExpected output:0,1,NORTHExample bPLACE 0,0,NORTHLEFTREPORTExpected output:0,0,WESTExample cPLACE 1,2,EASTMOVEMOVELEFTMOVEREPORTExpected output3,3,NORTHDeliverablesThe Ruby source files, the test data and any test code.It is not required to provide any graphical output showing the  movement of the toy robot.GitHub (Please refer to see the whole project)cli.rbmodule Robot  class CLI    def initialize      options = {:file => nil , :x => 5 , :y => 5}      parser = OptionParser.new do|opts|        opts.banner = Usage: toyrobot [options]        opts.on('-f', '--file filepath', 'Filepath for input commands') do |filename|          options[:file] = filename        end        opts.on('-x', '--xcoordinate X', 'Max X co-ordinate(Number)') do |max_x|          begin            options[:x] =  Integer(max_x)          rescue            puts Invalid x argument            puts opts            exit          end          end        opts.on('-y', '--ycoordinate Y', 'Max Y co-ordinate(Number)') do |max_y|          begin           options[:y] =  Integer(max_y)          rescue            puts Invalid y argument            puts opts            exit          end         end        opts.on('-h', '--help', 'Displays Help') do          puts opts          exit        end     end        parser.parse!      @application = Robot::App.new(options)    end    def start      @application.start    end  endendapp.rbmodule Robot class App   def initialize(opts)     @input_file = opts[:file].nil? ?  STDIN : File.open(opts[:file])      @simulator  = Robot::Simulator.new opts[:x], opts[:y]   end   def start          command = read_command     while (command) do         $logger.debug(Received command #{command})       begin        @simulator.execute command       rescue => e         $logger.error(e)       end       command = read_command     end   end   def read_command     print #  if @input_file == STDIN     command = @input_file.gets     exit if command.nil? || (command.chomp.downcase == .quit)     command     end endendcommand_parser.rbmodule Robot  class CommandParser    @@number_reg = /^\d+$/    # allowed_commnads is a hash     # with value as :method => [[arg types],[regex_range_for_strings_only]]    def initialize(allowed_commands)      @allowed_commands = allowed_commands      $logger.info(Allowed commands are #{@allowed_commands.keys})    end    def parse command        $logger.debug(Parsing command #{command})               args = command.split          method = args.delete_at(0)        if valid?(method, args)             update_args! method , args          yield method , args          return true        else          $logger.warn(Parsing failed. Invalid #{command})          return false        end    end   private     def update_args! method , args            @allowed_commands[method][0].each_with_index do |arg_type,i|         case arg_type           when :number             args[i] = args[i].to_i            when :string         end       end     end     def valid? (method , args)        return false unless @allowed_commands.has_key? method       unless  @allowed_commands[method].nil?         return false unless @allowed_commands[method][0].size == args.size         @allowed_commands[method][0].each_with_index do |arg_type,i|           case arg_type             when :number               return false unless args[i] =~ @@number_reg             when :string               allowed_reg = @allowed_commands[method][1][i]               unless allowed_reg.nil?                 return false unless args[i] =~ /#{allowed_reg}/               end            end              end        end        return true      end  endenddirection.rbmodule Robot    class Direction     attr_accessor :direction   def initialize(direction)     @direction = direction   end   @NORTH = Direction.new NORTH   @SOUTH = Direction.new SOUTH   @EAST  = Direction.new EAST   @WEST  = Direction.new WEST   @CLOCKWISE_DIRECTIONS = [@NORTH,@EAST,@SOUTH,@WEST]   def to_s      @direction   end   class << self     attr_accessor :NORTH, :SOUTH, :EAST, :WEST     end   def self.find(direction)     @CLOCKWISE_DIRECTIONS.find{|d| d.direction == direction.upcase }   end   def self.left(direction)      @CLOCKWISE_DIRECTIONS[( @CLOCKWISE_DIRECTIONS.index(direction) - 1 ) % 4]     end    def self.right(direction)      @CLOCKWISE_DIRECTIONS[( @CLOCKWISE_DIRECTIONS.index(direction) + 1 ) % 4]     end endendposition.rbmodule Robot  class Position    attr_accessor :x, :y, :direction    def initialize(x,y,direction)      @x = x      @y = y      @direction = direction    end    def to_s      X= #{@x} Y=#{@y} Facing=#{@direction.to_s}    end     def equal?(another_position)      self.x == another_position.x &&      self.y == another_position.y &&      self.direction == another_position.direction     end    def move             curr_postion   = self.dup       case curr_postion.direction         when Direction.NORTH           curr_postion.y +=1            when Direction.SOUTH           curr_postion.y -=1          when Direction.EAST           curr_postion.x +=1          when Direction.WEST           curr_postion.x -=1          end       curr_postion    end    def left      curr_postion   = self.dup      curr_postion.direction = Direction.left @direction      curr_postion    end    def right      curr_postion   = self.dup      curr_postion.direction = Direction.right @direction      curr_postion    end  endendsimulator.rbmodule Robot  class Simulator     attr_accessor :toy_robot     def initialize max_x, max_y       commands = {                   PLACE  => [                                 [:number , :number , :string],                                 [nil,nil,^NORTH$|^SOUTH$|^EAST$|^WEST$]                               ],                    MOVE   => [[],[]],                    LEFT   => [[],[]],                   RIGHT  => [[],[]],                    REPORT => [[],[]]                   }       @command_parser = CommandParser.new(commands)       @table          = Table.new max_x , max_y       @toy_robot      = ToyRobot.new       $logger.info Simulator created successfully.     end     def execute(command)        r = @command_parser.parse(command) do |method,args|          $logger.debug(#{method.downcase} - args #{args})          self.send( method.downcase , * args)        end        $logger.debug(@toy_robot.to_s)     end     def place x , y , face       if @table.inside?(x, y)         @toy_robot.position = Position.new(x, y, Direction.find(face))         @toy_robot.placed   = true       end     end     def move       return unless @toy_robot.placed            next_position =  @toy_robot.position.move       if @table.inside? next_position.x , next_position.y         @toy_robot.position = next_position       else         ignore       end     end     def left       return unless @toy_robot.placed         @toy_robot.position = @toy_robot.position.left       end     def right       return unless @toy_robot.placed       @toy_robot.position = @toy_robot.position.right     end     def report       if @toy_robot.placed         puts #{@toy_robot.position.x} #{@toy_robot.position.y} #{@toy_robot.position.direction}       else         puts Robot is not placed yet. Please use PLACE command to place the robot.       end     end     def ignore       $logger.debug Ignored step towards #{toy_robot.position.direction}     end  endendtable.rbmodule Robot  class Table     def initialize max_x , max_y        @MAX_X = max_x        @MAX_Y = max_y        $logger.info Table boundaries are #{@MAX_X},#{@MAX_Y}     end     def inside? x,y       return ((0..@MAX_X-1) === x) && ((0..@MAX_Y-1) === y)      end  endendtoyrobot.rbmodule Robot  class ToyRobot    attr_accessor :position, :placed    def initialize      @position  = nil      @placed    = false      $logger.info Toy Robot created successfully.    end    def to_s      if @placed        Placed at #{@position.to_s}      else        Not placed      end    end  endendrobot.rb$LOAD_PATH << File.join(File.dirname(__FILE__))require optparserequire logger require robot/versionrequire 'robot/command_parser'require 'robot/table'require 'robot/position'require 'robot/toy_robot'require robot/directionrequire robot/simulatorrequire robot/apprequire robot/cli$logger = Logger.new('log/toy_robot.log')Test codecommand_parser_spec.rbrequire_relative '../spec_helper'require_relative '../../lib/robot'include Robotdescribe CommandParser do  subject(:command_parser) {CommandParser.new({                   Salute => [                                 [:string],                                 [^Hello$|^Namaste$]                                ],                    Name => [                                 [:string],                                 []                              ] ,                    Age => [                                 [:number],                                 []                              ]                                    })}  context #with valid command do      context with range do      before {                @ran    = false                @called = command_parser.parse Salute Hello do |m,a|                  @ran = true                  @m = m                  @a = a                end      }      it { expect(@ran).to    be true }      it { expect(@called).to be true }        it { expect(@a.size).to be 1    }       it { expect(@a[0]).to  match Hello  }       it { expect(@m).to     match Salute }          end  context without range do        context with string do         before {                @ran    = false                @called = command_parser.parse Name Tom do |m,a|                  @ran = true                  @m = m                  @a = a                end               }       it { expect(@ran).to be true    }       it { expect(@called).to be true }       it { expect(@a.size).to be 1    }        it { expect(@a[0]).to  match Tom  }        it { expect(@m).to     match Name }         end     context with number do       before {                @ran = false                @called = command_parser.parse Age 50 do |m,a|                          @ran    = true                          @m      = m                          @a      = a                end       }       it { expect(@ran).to    be true }       it { expect(@called).to be true }       it { expect(@a.size).to be 1    }          it { expect(@a[0]).to   be 50   }        it { expect(@m).to     match      Age}         end    end      end        context #with invalid command do        before {                @ran = false                @called = command_parser.parse Salute Hi do |m,a|                  @ran = true                end      }     it { expect(@called).to be false }     it { expect(@called).to be false }            endenddirection_spec.rbrequire_relative '../spec_helper'require_relative '../../lib/robot'include Robotdescribe Direction do  describe :: finds directions do     it { expect( Direction.find NORTH ).to be Direction.NORTH }     it { expect( Direction.find SOUTH ).to be Direction.SOUTH }     it { expect( Direction.find EAST  ).to be Direction.EAST  }     it { expect( Direction.find WEST  ).to be Direction.WEST  }  end  describe :: turns left do     it { expect( Direction.left Direction.NORTH ).to be Direction.WEST }     it { expect( Direction.left Direction.SOUTH ).to be Direction.EAST }     it { expect( Direction.left Direction.EAST ).to be Direction.NORTH }     it { expect( Direction.left Direction.WEST ).to be Direction.SOUTH }  end  describe :: turns right do     it { expect( Direction.right Direction.NORTH ).to be Direction.EAST }     it { expect( Direction.right Direction.SOUTH ).to be Direction.WEST }     it { expect( Direction.right Direction.EAST ).to be Direction.SOUTH }     it { expect( Direction.right Direction.WEST ).to be Direction.NORTH }  endendposition_spec.rbrequire_relative '../spec_helper'require_relative '../../lib/robot'include Robotdescribe Position do  context #moves correctly to north do    before {      @position      = Position.new(2, 3, Direction.NORTH)      @curr_position = @position.move    }    it { expect(@curr_position.x).to be 2 }    it { expect(@curr_position.y).to be 4 }    it { expect(@curr_position.direction).to be Direction.NORTH }  end  context #moves correctly to south do    before {      @position      = Position.new(2, 3, Direction.SOUTH)      @curr_position = @position.move    }    it { expect(@curr_position.x).to be 2 }    it { expect(@curr_position.y).to be 2 }    it { expect(@curr_position.direction).to be Direction.SOUTH }  end  context #moves correctly to east do    before {      @position      = Position.new(2, 3, Direction.EAST)      @curr_position = @position.move    }    it { expect(@curr_position.x).to be 3 }    it { expect(@curr_position.y).to be 3 }    it { expect(@curr_position.direction).to be Direction.EAST }  end  context #moves correctly to west do    before {      @position      = Position.new(2, 3, Direction.WEST)      @curr_position = @position.move    }    it { expect(@curr_position.x).to be 1 }    it { expect(@curr_position.y).to be 3 }    it { expect(@curr_position.direction).to be Direction.WEST }  end  context #turns left do    before {      start_position  = Position.new(2, 3, Direction.WEST)      @position  = start_position.left    }    it { expect(@position.x).to be 2 }    it { expect(@position.y).to be 3 }    it { expect(@position.direction).to be Direction.SOUTH }  end  context #turns right do    before {      start_position  = Position.new(2, 3, Direction.WEST)      @position       = start_position.right    }    it { expect(@position.x).to be 2 }    it { expect(@position.y).to be 3 }    it { expect(@position.direction).to be Direction.NORTH }  endendsimualtor_spec.rbrequire_relative '../spec_helper'require_relative '../../lib/robot'include Robotdescribe Simulator do  describe #gets placed do    before {      @simulator = Simulator.new 5,5      @simulator.place 2, 3, NORTH    }    it { expect(@simulator.toy_robot.position).to be Position.new(2,3,Direction.NORTH) }  end  describe #moves do    context when inside table do      before {        @simulator = Simulator.new 5,5        @simulator.place 2, 3, NORTH        @simulator.move      }      it { expect(@simulator.toy_robot.position).to be Position.new(2,4,Direction.NORTH) }    end    context when at edge of table do      before {        @simulator = Simulator.new 5,5        @simulator.place 4, 4, NORTH        @simulator.move      }      it { expect(@simulator.toy_robot.position).to be Position.new(4,4,Direction.NORTH) }    end  end  describe # turns do    context when faced north do      before {          @simulator = Simulator.new 5,5          @simulator.place 2, 2, NORTH             }      it { @simulator.left  ; expect(@simulator.toy_robot.position).to be Position.new(2,2,Direction.WEST) }      it { @simulator.right ; expect(@simulator.toy_robot.position).to be Position.new(2,2,Direction.EAST) }     end     context when faced south do      before {          @simulator = Simulator.new 5,5          @simulator.place 2, 2, SOUTH             }      it { @simulator.left  ; expect(@simulator.toy_robot.position).to be Position.new(2,2,Direction.EAST) }      it { @simulator.right ; expect(@simulator.toy_robot.position).to be Position.new(2,2,Direction.WEST) }     end      context when faced east do      before {          @simulator = Simulator.new 5,5          @simulator.place 2, 2, EAST             }      it { @simulator.left  ; expect(@simulator.toy_robot.position).to be Position.new(2,2,Direction.NORTH) }      it { @simulator.right ; expect(@simulator.toy_robot.position).to be Position.new(2,2,Direction.SOUTH) }     end       context when faced west do      before {          @simulator = Simulator.new 5,5          @simulator.place 2, 2, WEST             }      it { @simulator.left  ; expect(@simulator.toy_robot.position).to be Position.new(2,2,Direction.SOUTH) }      it { @simulator.right ; expect(@simulator.toy_robot.position).to be Position.new(2,2,Direction.NORTH) }     end    endendtable_spec.rbrequire_relative '../spec_helper'require_relative '../../lib/robot'include Robotdescribe Table do  context #checks boundry do    before { @t = Table.new 5,5 }    it {expect(@t.inside?(2,3)).to be true}    it {expect(@t.inside?(0,0)).to be true}    it {expect(@t.inside?(5,5)).to be false}    it {expect(@t.inside?(5,3)).to be false}    it {expect(@t.inside?(3,5)).to be false}    it {expect(@t.inside?(6,3)).to be false}    it {expect(@t.inside?(3,6)).to be false}    it {expect(@t.inside?(-1,0)).to be false}    it {expect(@t.inside?(0,-1)).to be false}  end  end
Robot toy simulator
ruby;unit testing;interview questions;state machine
Initial impressionsThere's a lot of code.  Interviewers don't like to read a lot of code.In many ways, you have overdelivered on the specification.  Since you have indicated that this was an interview-question, I'm going to give you my personal value judgements on these points.I'm ambivalent about the logger.  On one hand, it is a nice debugging aid.  On the other hand, the spec just says that any invalid instruction is to be ignored, so all of those infos, warnings, and errors are unnecessary complications.  The spec certainly did not ask for a log/toy_robot.log file to be created by default, so that was a bit of a surprise.Do you really need to log Toy Robot created successfully and Simulator created successfully at the info level?  I'd consider those to be debug-level messages, and even then, I'd wonder what could possibly go wrong in such a simple program that the ToyRobot and Simulator would fail to be instantiated.Instead of logging warnings and errors, you could have just thrown exceptions, catching and reporting them in one exception handler.The very first requirement states that the table is a 55 square.  Why are you bothering to accept -x and -y flags to specify the width and height?  You could have ditched the entire cli.rb file, accepting no command-line options, and just read from ARGF to satisfy the requirement that the input can come from a file or from standard input.I'm not convinced that all of those classes need to exist.As I mentioned, OptionParser isn't called for, so the CLI class can go away.Why is App a separate class from CLI?  What about Simulator?  Isn't this whole project a simulator app?The ToyRobot class looks more like a dumb struct rather than a smart object.The code to support rotations and translations is split up between Direction and Position.What is Position#equal? used for?  What about Position#to_s  couldn't you have defined it strategically so that it can be reused for the REPORT command?So, there is some overengineering going on, and some bloat, acting together to produce a solution that is larger than it needs to be.Second impressionsYou do seem familiar with Ruby.  For example, you used ranges and === to perform the bounds check instead of 0 <= x && x < @MAX_X && 0 <= y && y < @MAX_Y. def inside? x,y   return ((0..@MAX_X-1) === x) && ((0..@MAX_Y-1) === y)  endOn the other hand, it would have been slightly prettier with exclusive ranges, and without the explicit return: def inside?(x, y)   (0...@MAX_X) === x && (0...@MAX_Y) === y endThe Direction enum class is not bad  it demonstrates familiarity with metaprogramming.  The attr_accessors would be better as attr_readers, since they should all be constant.Command dispatchingThis is the main point of the exercise.You've split up the work between App, CommandParser and Simulator.  Within the Simulator constructor, there is a command hash that is mostly there to support the PLACE command parameters, and you're repeating yourself by having to list those commands explicitly.In App#read_command, instead of calling exit, I suggest a more disciplined throw-catch.Suggested solutionFor comparison, this is what I came up with.class Direction  def initialize(sym, dx, dy, left, right)    @name, @dx, @dy, @left, @right = sym.to_s, dx, dy, left, right  end  def to_s    @name  end  def left    Direction.const_get(@left)  end  def right    Direction.const_get(@right)  end  def advance(x, y)    [x + @dx, y + @dy]  end  @all = [    [:EAST,  +1,  0],    [:NORTH,  0, +1],    [:WEST,  -1,  0],    [:SOUTH,  0, -1],  ]  @all.each_with_index do |(sym, dx, dy), i|    self.const_set(sym,                   Direction.new(sym, dx, dy, @all[(i + 1) % @all.size].first,                                              @all[(i - 1) % @all.size].first))  end  def self.[](name)    Direction.const_get(name) if Direction.const_defined?(name)  endendclass InvalidCommand < Exception ; end# Mixin for Robotmodule Commands  def place(x, y, face)    validate(x = (x.to_i if /\A\d+\Z/.match(x)),             y = (y.to_i if /\A\d+\Z/.match(y)),             face = Direction[face.upcase])    @x, @y, @face = x, y, face  end  def move    raise InvalidCommand.new unless @face    new_x, new_y = @face.advance(@x, @y)    validate(new_x, new_y, @face)    @x, @y = new_x, new_y  end  def left    raise InvalidCommand.new unless @face    @face = @face.left  end  def right    raise InvalidCommand.new unless @face    @face = @face.right  end  def report    raise InvalidCommand.new unless @face    @output.puts #{@x},#{@y},#{@face}  endendclass Robot  include Commands  def initialize(board_size=5, output=STDOUT)    @board_size = board_size    @output = output  end  def execute(cmd)    cmd = cmd.strip.downcase    op, args = cmd.split(/\s+/, 2)    args = args.split(/\s*,\s*/) if args    raise InvalidCommand.new unless Commands.public_method_defined?(op)    begin      send(op, *args)    rescue ArgumentError      raise InvalidCommand.new    end  end  def execute_script(io)    io.each_line do |line|      begin        execute(line)      rescue InvalidCommand        #puts (Ignored invalid command)      end    end  end  private  def validate(x, y, f)    raise InvalidCommand.new unless x && y && f    raise InvalidCommand.new unless (0..@board_size) === x    raise InvalidCommand.new unless (0..@board_size) === y  endendRobot.new.execute_script(ARGF)