1
\$\begingroup\$

I recently didn't pass a take-home code test for a job interview - and one of the reasons for this was that the code was not as idiomatic as it could be.

The premise is in the title - but below details the instructions in the readme to get an idea of how it works typing in commands to get desired result:

I N M - Create a new M x N image with all pixels coloured white (O).

L X Y C - Colours the pixel (X,Y) with colour C.

V X Y1 Y2 C - Draw a vertical segment of colour C in column X between rows Y1 and Y2 (inclusive).

The code works and all tests pass (don't want to just paste the entire project on here) - but I would like to get feedback on whether the approach I've taken (i.e - OOP approach having 2 classes and updating their attributes throughout / using a case statement to accept input and validate with regex) is good practice - and anywhere where I may have slipped away from a native and idiomatic ruby approach.

Many thanks in advance for any feedback.

This is the code for the main bitmap editor program that runs and accepts the commands:

class BitmapEditor
  require 'pry'
  require './lib/bitmap_matrix'
  attr_accessor :running

  def initialize(running)
    @running = running
  end

  def run
    puts "Please enter your command here"
    while @running
      # STORE VARIABLE FOR USER INPUT
      command = STDIN.gets.chomp
      # EVALUATE CASE STATEMENT
      evaluate_command(command)
    end

  end

  # CREATE CASE STATEMENT TO RECOGNIZE THE INPUT
  def evaluate_command(command)
    puts "evaluating command"
    # CHECK IF BITMAP EXISTS IF THE COMMAND IS NOT TO CREATE A BITMAP OR QUIT PROGRAM
    if bitmap_required(command.rstrip)
      # STRIP END SPACE FROM COMMAND
      case command.rstrip

      when /I\s\d+\s\d+\z/
        @bitmap_matrix = create_bitmap(*split_command(command))
      when 'Q'
        puts 'Quitting Program'
        @running = false
      when 'C'
        @bitmap_matrix&.clear
      when /L\s\d+\s\d+\s[a-zA-Z]\z/
        @bitmap_matrix&.color_single_pixel(*split_command(command))
      when /V\s\d+\s\d+\s\d+\s[a-zA-Z]\z/
        @bitmap_matrix&.color_vertical_segment(*split_command(command)) 
      when /H\s\d+\s\d+\s\d+\s[a-zA-Z]\z/
        @bitmap_matrix&.color_horizontal_segment(*split_command(command))
      when 'S'
        @bitmap_matrix&.draw
      else 
        puts 'Command not recognized!'
      end
    else 
      puts "Please draw Bitmap first"
    end
  end

  def create_bitmap(rows, columns)
    @bitmap_matrix = BitmapMatrix.new(rows.to_i, columns.to_i)
  end

  # METHOD TO DYNAMICALLY TAKE THE PARAMETERS OF COMMAND AFTER FIRST LETTER AND CONVERT THEM INTO ARGUMENTS FOR PROCESSING BITMAP QUERY
  def split_command(command)
    stripped = command.split(" ")
    stripped.delete(stripped[0])
    stripped.map{|char| self.is_numeric?(char)}
  end

  def bitmap_required(command)
    command_doesnt_require_bitmap = command.match(/I\s\d+\s\d+\z/) || command == "Q"
    @bitmap_matrix || command_doesnt_require_bitmap
  end

  #IF COMMAND CHAR IS AN INTEGER - CONVERT TO INTEGER TO BE COMPATIBLE WITH BITMAP MATRIX
  def is_numeric?(char)
    char =~ /[0-9]/ ? char.to_i : char
  end
end

This is the bitmap matrix class that builds the bitmap and applies the instructions specified in the above class:

class BitmapMatrix
  attr_accessor :row_count, :column_count, :matrix, :valid_request

  def initialize(columns, rows)
    @row_count = rows
    @column_count = columns
    @matrix = row_initialize(columns, rows)
    @valid_request = true
    puts 'BitMap created!'
  end

  def draw 
    @matrix.values.map{|row| puts row.join(" ")}
  end

  def color_single_pixel(x, y, color)
    # VALIDATE COORDINATE ARE IN RANGE AND REASSIGN SPECIFIC VALUE IN HASH TO REPRESENT PIXEL COLORING
    if @matrix[y]
      @matrix[y][x-1] ?  @matrix[y][x-1] = color : invalid_coordinates
      puts "Pixel Drawn"
    else 
      invalid_coordinates
    end
  end

  def color_vertical_segment(column, y1, y2, color)
    # VALIDATE WHETHER GIVEN COLUMS AND ROWS ARE IN RANGE
    if @matrix[y1] && @matrix[y2]
      valid = (@matrix[y1][column-1] && @matrix[y2][column-1])
      # LOOP GIVEN RANGE AND REASSIGN HASH VALUE TO REPRESENT PIXEL DRAWN
      valid ? line_drawing(y1, y2, 'row', column, color) : invalid_coordinates
    else 
      invalid_coordinates
    end
  end

  def color_horizontal_segment( x1, x2, row, color)
    # VALIDATE WHETHER GIVEN COLUMS AND ROWS ARE IN RANGE
    if @matrix[row]
      valid = (@matrix[row][x1-1] && @matrix[row][x2-1])
      # LOOP GIVEN RANGE AND REASSIGN HASH VALUE TO REPRESENT PIXEL DRAWN
      valid ? line_drawing(x1, x2, 'column', row, color) : invalid_coordinates
    else 
      invalid_coordinates
    end
  end
  # REINITIALIZING GRID HASH RESETS ALL TO O
  def clear 
    @matrix = row_initialize(@column_count, @row_count)
    puts 'Grid cleared!'
  end

  def line_drawing(c1, c2, row_or_column, static_value, color)
    static_coordinate = row_or_column == "row" ? true : false
    sorted = [c1, c2].sort
    (sorted[0]..sorted[1]).to_a.map{|row_or_column| static_coordinate ? @matrix[row_or_column][static_value-1] = color : @matrix[static_value][row_or_column-1] = color}
    puts 'Pixel Line Drawn'
  end


  #CREATE A HASH THAT REPRESENTS ALL THE GRID POINTS
  def row_initialize(columns, rows)
    @matrix= {}
    rows.times do |row|
      row_array = []
      columns.times{|n| row_array.push("O")}
      @matrix[row+1] = row_array
    end 
    @matrix 
  end

  def invalid_coordinates 
    puts "Not valid coordinates"
  end
end
\$\endgroup\$

Your Answer

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

Browse other questions tagged or ask your own question.