HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Year 0: Instruction Follower

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
followerinstructionyear

Problem

Having played Human Resource Machine for over a day, and wanting to improve my interpreting / tokenizer skills, I made a Human Resource Machine interpreter.

For those that do not know, so if you do you can skip this. Human Resource Machine is a game where you're an 'instruction follower' employee.
You are given tasks to complete, things such as multiplying numbers, returning the smaller word, counting from a number to zero.
To complete these task you create a program constructed from a fairly small list of functions, tiles on the floor and an input and output queue.
To give an example the first level you are given two commands 'inbox' and 'outbox', and the task is to pass three boxes with data in them to the output.
In the second level you are introduced to the 'jump' command which is the same as using labels and goto in languages that support it,
allowing you to pass any number of input to the output.
There are then:

  • 'copyfrom' and 'copyto' that copy to or from the tiles on the floor from/to your hand.



  • 'add' and 'sub' that apply the respective function on the box in you hand and the tile on the floor. If your hand is 5, and you sub a tile with the value 4, you'll result in having 1 in your hand.



  • 'bump+' and 'bump-' which increase and decrease the value in that space by one, and add the item to your hand. And,



  • 'jump if zero' and 'jump if negative' which only jump if the value in your hand is zero or negative.



Sometimes levels can also start with boxes already in tiles. All of this is how your average Joe would interact with the game, but this one heavily abused the copy ability in the game that gives you output such as:

-- HUMAN RESOURCE MACHINE PROGRAM --

a:
    INBOX   
    COPYTO   0
b:
c:
    OUTBOX  
    COPYFROM 0
    JUMPZ    a
    JUMPN    d
    BUMPDN   0
    JUMP     b
d:
    BUMPUP   0
    JUMP     c


This takes a number and adds or minuses one, outputs it, and loops until the number is zero.
Before I bore you there's one o

Solution

I agree with your feeling about the design. I wouldn't necessarily say it's wrong, but it feels a little heavy for the task at hand.

Firstly, you have quite a lot of code that's slightly tricky, or "strange" as SuperBiasedMan said. As his answer also said, we could benefit a lot from some comments explaining what your methods and classes do, especially in places where the intent of the code may not immediately obvious to somebody reading it for the first time.

With no disrespect intended, can I make a guess that you're coming to Python from a background in a statically-typed, strongly object-oriented language like Java? I really like Python as a language, and one reason is that you can get a lot done with a minimum of fuss and boilerplate. I think it's a good choice for this task, but personally, I probably wouldn't use so many classes. Plain old lists, dictionaries, ints and strings will take you a long way most cases.

The language you're interpreting seems to be a very simple assembly language. Your interpreter is essentially a mapper from textual commands and arguments to simple operations. Sounds like your input is guaranteed to be valid, since it's copy and pasted from the game UI. I understand you may be trying to make a more complete interpreter with strict checking as an exercise, but why not start more simply?

With that in mind, how about something like this as a starting point? I tested it on your simple example, but haven't run it on all your unit tests. I may be missing some aspects of the game or language, and it doesn't have detailed input checking, but it's a basic working interpreter using 100 lines of code rather than 300.

import sys

class Instruction:
  def __init__(self, instr, arg=None):
    self.instr = instr
    self.arg = arg

def parse(hr_program_lines):
  instructions = []    # list of instructions
  labels = {}          # map labels to instruction indexes
  for line in hr_program_lines:
    if line.startswith((' ', '\t')):
      split = line.strip().split()
      instr = Instruction(split[0], split[1] if len(split) > 1 else None)
      instructions.append(instr)
    else:
      label = line.strip().rstrip(':')
      # point the label at the next instruction to be parsed
      labels[label] = len(instructions)
  return instructions, labels

class Interpreter:
  def __init__(self, instructions, labels, output_fn=print):
    self.instructions = instructions
    self.labels = labels
    self.output_fn = output_fn

  def _eval_int(self, arg):
    if arg is None:
      return None
    try:    # Integer value
      return int(arg)
    except ValueError:
      try:  # Pointer
        ptr = int(arg.strip('[]'))
        return self.tiles[int(ptr)]
      except ValueError:
        # String to integer - A -> 1, B -> 2 etc
        return ord(arg[0].upper()) - 64

  def inbox(self, _arg):
    # Will throw StopIteration if all input is consumed
    self.hand = self._eval_int(next(self.input_iter))

  def outbox(self, _arg):
    self.output_fn(self.hand)

  def copyfrom(self, idx):
    self.hand = self.tiles[idx]

  def copyto(self, idx):
    self.tiles[idx] = self.hand

  def bumpdn(self, idx):
    self.tiles[idx] -= 1
    self.hand = self.tiles[idx]

  def add(self, n):
    self.hand += self.tiles[n]

  def jump(self, addr):
    self.ip = addr

  def jumpz(self, addr):
    if 0 == self.hand:
      self.ip = addr

  def jumpn(self, addr):
    if self.hand < 0:
      self.ip = addr

  def execute_step(self):
    instr = self.instructions[self.ip]
    # print("  IP: %d" % self.ip)
    # print("  hand:  %s" % str(self.hand))
    # print("  tiles: %s" % self.tiles)
    # print(" Executing %s" % instr.instr)
    method_name = instr.instr.lower()
    try:
      method = getattr(self, method_name)
    except AttributeError:
      raise(Exception('Unrecognized instruction %s' % instr.instr))
    if method_name.startswith('jump'):
      method(self.labels[instr.arg])
    else:
      method(self._eval_int(instr.arg))
      self.ip += 1

  def execute(self, inputs):
    # print('EXECUTING PROGRAM ON INPUT %s' % str(inputs))
    self.ip = 0     # instruction pointer
    self.tiles = {} # indexed by integer address
    self.hand = None
    self.input_iter = iter(inputs)
    try:
      while True:
        self.execute_step()
    except StopIteration:
      # print('EXECUTION COMPLETE')
      return # all input consumed

if '__main__' == __name__:
  instructions, labels = parse(sys.stdin.readlines())
  interpreter = Interpreter(instructions, labels)
  interpreter.execute([1, 2])
  interpreter.execute(['A', 'B'])


Having read your code again in more detail, here's a short list of things I liked and didn't like so much about it:

Liked:

  • Your idea to write the interpreter in the first place, and the fact that you put it up for review!



  • Methods named after instructions, logical use of metaprogramming



  • Natural use of iter() and StopIteration



  • Unit tests! +10 for this :)



Didn't like so muc

Code Snippets

import sys

class Instruction:
  def __init__(self, instr, arg=None):
    self.instr = instr
    self.arg = arg


def parse(hr_program_lines):
  instructions = []    # list of instructions
  labels = {}          # map labels to instruction indexes
  for line in hr_program_lines:
    if line.startswith((' ', '\t')):
      split = line.strip().split()
      instr = Instruction(split[0], split[1] if len(split) > 1 else None)
      instructions.append(instr)
    else:
      label = line.strip().rstrip(':')
      # point the label at the next instruction to be parsed
      labels[label] = len(instructions)
  return instructions, labels


class Interpreter:
  def __init__(self, instructions, labels, output_fn=print):
    self.instructions = instructions
    self.labels = labels
    self.output_fn = output_fn

  def _eval_int(self, arg):
    if arg is None:
      return None
    try:    # Integer value
      return int(arg)
    except ValueError:
      try:  # Pointer
        ptr = int(arg.strip('[]'))
        return self.tiles[int(ptr)]
      except ValueError:
        # String to integer - A -> 1, B -> 2 etc
        return ord(arg[0].upper()) - 64

  def inbox(self, _arg):
    # Will throw StopIteration if all input is consumed
    self.hand = self._eval_int(next(self.input_iter))

  def outbox(self, _arg):
    self.output_fn(self.hand)

  def copyfrom(self, idx):
    self.hand = self.tiles[idx]

  def copyto(self, idx):
    self.tiles[idx] = self.hand

  def bumpdn(self, idx):
    self.tiles[idx] -= 1
    self.hand = self.tiles[idx]

  def add(self, n):
    self.hand += self.tiles[n]

  def jump(self, addr):
    self.ip = addr

  def jumpz(self, addr):
    if 0 == self.hand:
      self.ip = addr

  def jumpn(self, addr):
    if self.hand < 0:
      self.ip = addr

  def execute_step(self):
    instr = self.instructions[self.ip]
    # print("  IP: %d" % self.ip)
    # print("  hand:  %s" % str(self.hand))
    # print("  tiles: %s" % self.tiles)
    # print(" Executing %s" % instr.instr)
    method_name = instr.instr.lower()
    try:
      method = getattr(self, method_name)
    except AttributeError:
      raise(Exception('Unrecognized instruction %s' % instr.instr))
    if method_name.startswith('jump'):
      method(self.labels[instr.arg])
    else:
      method(self._eval_int(instr.arg))
      self.ip += 1

  def execute(self, inputs):
    # print('EXECUTING PROGRAM ON INPUT %s' % str(inputs))
    self.ip = 0     # instruction pointer
    self.tiles = {} # indexed by integer address
    self.hand = None
    self.input_iter = iter(inputs)
    try:
      while True:
        self.execute_step()
    except StopIteration:
      # print('EXECUTION COMPLETE')
      return # all input consumed

if '__main__' == __name__:
  instructions, labels = parse(sys.stdin.readlines())
  interpreter = Interpreter(instructions, labels)
  interpreter.execute([1, 2])
  interpreter.execute(['A', 'B'])

Context

StackExchange Code Review Q#132275, answer score: 7

Revisions (0)

No revisions yet.