IntCode computer in Python 3

Question

I'm using advent of code 2019 to start learning Python. I've read the Python style guide, and I'm trying to write readable and reusable code.

This piece of code is an implementation of the IntCode computer used in many of the assignments. If you'd like to see code for the specific assignments, it's on my Github.

Here's the code for the IntCode computer itself. Any suggestions for improvement are welcome, especially relating to the 'pythonic' way of doing things. For reference, my background is in Java.

import threading
from typing import List


class IntCoder:
    _memory = []
    __pointer = 0
    __relative_base = 0

    def __init__(self, memory: List[int]):
        self._memory = memory

    def run(self):
        while self.__step():
            pass

    def get_input(self):
        raise ValueError('No input defined')

    def handle_output(self, value):
        print(value)

    def read_pointer(self):
        return self.read(self.__pointer)

    def read(self, index):
        return self._memory[index]

    def __step(self):
        instruction = self._memory[self.__pointer]
        opcode = instruction % 100
        mode1 = (instruction // 100) % 10
        mode2 = (instruction // 1000) % 10
        mode3 = instruction // 10000
        if opcode == 1:
            self.__add(self.__val(self.__pointer + 1, mode1), self.__val(self.__pointer + 2, mode2), self.__index(self.__pointer + 3, mode3))
        elif opcode == 2:
            self.__mul(self.__val(self.__pointer + 1, mode1), self.__val(self.__pointer + 2, mode2), self.__index(self.__pointer + 3, mode3))
        elif opcode == 3:
            self.__in(self.__index(self.__pointer + 1, mode1))
        elif opcode == 4:
            self.__out(self.__val(self.__pointer + 1, mode1))
        elif opcode == 5:
            self.__if_true(self.__val(self.__pointer + 1, mode1), self.__val(self.__pointer + 2, mode2))
        elif opcode == 6:
            self.__if_false(self.__val(self.__pointer + 1, mode1), self.__val(self.__pointer + 2, mode2))
        elif opcode == 7:
            self.__lt(self.__val(self.__pointer + 1, mode1), self.__val(self.__pointer + 2, mode2), self.__index(self.__pointer + 3, mode3))
        elif opcode == 8:
            self.__eq(self.__val(self.__pointer + 1, mode1), self.__val(self.__pointer + 2, mode2), self.__index(self.__pointer + 3, mode3))
        elif opcode == 9:
            self.__arb(self.__val(self.__pointer + 1, mode1))
        elif opcode == 99:
            return False
        else:
            raise ValueError('Unknown opcode {} in instruction {}'.format(opcode, instruction))
        return True

    def __val(self, address, mode):
        index = self._memory[address]
        if mode == 0:
            return self._memory[index]
        elif mode == 1:
            return index
        elif mode == 2:
            return self._memory[index + self.__relative_base]
        else:
            raise ValueError('Unknown parameter mode {}'.format(mode))

    def __index(self, address, mode):
        if mode == 0:
            return self._memory[address]
        elif mode == 2:
            return self._memory[address] + self.__relative_base
        raise ValueError('Got mode {} for index only type param'.format(mode))

    def __add(self, first, second, result):
        self._memory[result] = first + second
        self.__pointer += 4

    def __mul(self, first, second, result):
        self._memory[result] = first * second
        self.__pointer += 4

    def __in(self, index):
        self._memory[index] = self.get_input()
        self.__pointer += 2

    def __out(self, value):
        self.handle_output(value)
        self.__pointer += 2

    def __if_true(self, first, second):
        self.__pointer = second if first != 0 else self.__pointer + 3

    def __if_false(self, first, second):
        self.__pointer = second if first == 0 else self.__pointer + 3

    def __lt(self, first, second, result):
        self._memory[result] = 1 if first < second else 0
        self.__pointer += 4

    def __eq(self, first, second, result):
        self._memory[result] = 1 if first == second else 0
        self.__pointer += 4

    def __arb(self, value):
        self.__relative_base += value
        self.__pointer += 2

    @staticmethod
    def extended_memory(program, size):
        memory = [0] * size
        memory[0:len(program)] = program
        return memory

    @staticmethod
    def read_file(file):
        with open(file, 'r') as f:
            return [int(s) for s in f.read().split(',')]


class IntCoderWithIo(IntCoder):
    def __init__(self, memory, input_values: List[int]):
        super(IntCoderWithIo, self).__init__(memory)
        self.input_values = (n for n in input_values)
        self.output = []

    def get_input(self):
        return next(self.input_values)

    def handle_output(self, value):
        self.output.append(value)

Answer 1

Unused Imports

You've imported threading, but you are not using it anywhere in this module.

Dunder

Dunder, or double-underscore, members have special meaning in Python. Specifically, the Python interpreter does a form of name-mangling, to prevent member names defined in the parent class from colliding with member names defined in a derived class. This also prevents the derived class from accessing members defined in the parent class. I doubt this is what you were intending here. Avoid creating your own double underscores members.

Class Variables

Java lets you initialize member variables outside the constructor; Python does not. What you are doing is ugly, dangerous, and will bite you down the road.

class IntCoder:

    _memory = []                 # Class member
    _pointer = 0                 # Class member
    _relative_base = 0           # Class member

    def __init__(self, memory: List[int]):
        self._memory = memory    # Instance member

This code creates 3 class member variables (Ie, static in Java terminology), and then the IntCoder() constructor creates one instance member, shadowing the first class member.

When Python looks up self.member, it first looks in the self object's dictionary for member. If that fails, it will also look in the class's dictionary. Assignments to self.member will always create member in the object instance.

Now consider:

    self._pointer += 4

If _pointer is defined in the instance object, this is interpreted as:

    self._pointer = self._pointer + 4

If _pointer is not defined in the instance object, but exists in the class, this is interpreted as looking up the class member, and creating the instance member:

    self._pointer = IntCoder._pointer + 4

Yikes! Mutating code interpretation!

You really want each IntCoder to have its own, independent state, so they each should create their own instance members in the constructor:

class IntCoder:

    def __init__(self, memory: List[int]):
        self._memory = memory 
        self._pointer = 0      
        self._relative_base = 0

Pointer Management

These next few items all go together ...

self.read_pointer()

Never called. Why is this here?

instruction fetching

    instruction = self._memory[self.__pointer]

Looks like that could have been instruction = self.read_pointer(). But ...

Separation of concerns

    def __add(self, first, second, result):
        self._memory[result] = first + second
        self.__pointer += 4

Whoa, whoa, hold the phone. Ok, I understand you are adding the values first and second together, and storing the result in memory at address result. But where the heck did that self.__pointer += 4 come from? And why 4?

        if opcode == 1:
            self.__add(self.__val(self.__pointer + 1, mode1), self.__val(self.__pointer + 2, mode2), self.__index(self.__pointer + 3, mode3))

Ok, looking back, we can see that we got a value using from self._pointer + 1, a second value using self._pointer + 2, and use self._pointer + 3 to get the address to store the result. So that explains 3 of the increment of 4...

        instruction = self._memory[self.__pointer]

Oh ya, and the instruction itself in the 4th value.

What is the IntCoder really doing? It is:

1. Fetching the opcode
2. Fetching the first operand, or index to it
3. Fetching the second operand, or index to it
4. Fetching the destination address or index

Or, to summarize, it is fetching, ... and implicit in the fetch is an increment of the pointer.

    def _fetch(self):
        value = self._memory[self._pointer]
        self._pointer += 1
        return value

Now you can use:

        instruction = self._fetch()

and know that if you called _fetch() again, the next value would be read, and then the next, and the next. No need to externally code self._pointer += 1 anymore. Let's do the same for _val():

    def _val(self, mode):
        index = self._fetch()
        if mode == 0:
            return self._memory[index]
        if mode == 1:
            return index
        if mode == 2:
            return self._memory[index + self._relative_base]
        raise ValueError(f"Unknown parameter mode {mode}")

Note: Python 3.6 introduced f-strings, which allow you to embed variables right in strings, rather than needing .format(...)

If you also added the _fetch() to _index, you'd could now write:

        if opcode == 1:
            self._add(self._val(mode1), self._val(mode2), self._index(mode3))

and

    def _add(self, first, second, result):
        self._memory[result] = first + second

Decoding Instructions

A large chain of if opcode == 0: elif opcode == 1: elif opcode == 2: elif opcode == 3: ... is an indication of doing something wrong, or at least the non-Pythonic way.

Methods are first class objects in Python, and can be used as values and stored in containers. Which means you can use a dictionary to decode and dispatch based on the opcode:

class IntCoder:

    def __init__(self):
        self._opcodes = { 1: self._add,
                          2: self._mul,
                          3: self._in,
                          ...
                        }

    def _step(self):
        instruction = self._fetch()
        opcode = instruction % 100
        mode1 = (instruction // 100) % 10
        mode2 = (instruction // 1000) % 10
        mode3 = instruction // 10000

        try:
            self._opcodes[opcode](mode1, mode2, mode3)
        except KeyError:
            raise ValueError('Unknown opcode {opcode} in instruction {instruction-1}')

    def _add(self, mode1, mode2, mode3):
        a = self._val(mode1)
        b = self._val(mode2)
        dst = self._index(mode3)
        self._memory[dst] = a + b

    def _in(self, mode1, mode2, mode3):
        index = self._index(mode1)
        self._memory[index] = self.get_input()

Note we've changed thing a bit. The responsibility for fetching the values to add, and the destination to store the results into has moved into the individual functions. Additionally, we are passing unnecessary information to the _in() method; it doesn't need the mode2 and mode3 values, but it needs those arguments since with this dispatch method, all the methods have to use a common parameter list.

Iterators

The following is creating and storing a generator expression, to walk through the list of values, returning the values one at a time:

self.input_values = (n for n in input_values)

Internally, for n in input_values is walking through input_values, return each value one at a time. It works because input_values is an iterable. We can retrieve an iterator for that iterable object and use that directly.

self.input_values = iter(input_values)

Then you could pass in any input_values: Iterable[int], instead of just lists.