Simple Language Interpreter

I've been playing around with Python off and on for about the past year and recently came up with the following 68 (was 62) lines. I think I'll try making a calculator out of it. I'd really like to know what readers here think of its attributes such as coding style, readability, and feasible purposefulness.

```
# notes: separate addresses from data lest the loop of doom cometh

class Interpreter:

def __init__(self):
self.memory = { }
self.dictionary = {"mov" : self.mov,
"put" : self.put,
"add" : self.add,
"sub" : self.sub,
"clr" : self.clr,
"cpy" : self.cpy,
"ref" : self.ref }
self.hooks = {self.val("0") : self.out }

def interpret(self, line):
x = line.split(" ")
vals = tuple(self.val(y) for y in x[1:])
dereferenced = []
keys_only = tuple(key for key in self.memory)
for val in vals:
while val in self.memory: val = self.memory[val]
dereferenced.append(val)
vals = tuple(y for y in dereferenced)
self.dictionary[x[0]](vals)

def val(self, x):
return tuple(int(y) for y in str(x).split("."))

def mov(self, value):
self.ptr = value[0]

def put(self, value):
self.memory[self.ptr] = value[0]

def clr(self, value):
if self.ptr in self.hooks and self.ptr in self.memory:
x = self.hooks[self.ptr]
y = self.memory[self.ptr]
for z in y: x(z)
del self.memory[self.ptr]

def add(self, values):
self.put(self.mat(values, lambda x, y: x + y))

def sub(self, values):
self.put(self.mat(values, lambda x, y: x - y))

def mat(self, values, op):
a, b = self.memory[values[0]], self.memory[values[1]]
if len(a) > len(b): a, b = b, a
c = [op(a[x], b[x]) for x in xrange(len(b))] + [x for x in a[len(a):]]
return [tuple(x for x in c)]

def cpy(self, value):
self.put(value)

def out(self, x):
print chr(x),

def ref(self

Sorry for bluntness, intended or not, but here we go!

Not very good readability IMO. It takes a while to understand what the code does. Unit tests or at least some sample input and output may make it obvious.

In general, naming should be improved. It's difficult to understand what a function or variable does from its name.

There's also much shuffling of data back and forth that seems unnecessary. It looks like this can be simplified quite a bit, see inline comments.

Also, you use tuples in many situations where a list would be more natural.

As I had problems understanding the code, I may have misunderstood things in it, so read the following inline comments with that in mind.

# notes: separate addresses from data lest the loop of doom cometh

Not a very helpful comment, is it?

class Interpreter:

Can you find a class name that better tells what this class does? It not only interprets.

def __init__(self):
    self.memory = { }

I'd like a comment on what self.memory is.

self.dictionary = {"mov" : self.mov,
                       "put" : self.put,
                       "add" : self.add,
                       "sub" : self.sub,
                       "clr" : self.clr,
                       "cpy" : self.cpy,
                       "ref" : self.ref }

No shit, it's a dictionary. But can you find a name that tells what kind of dictionary it is?

To reduce code size, you may consider self.dictionary = dict(self.getattr(attr) for attr in 'mov put add sub clr cpy ref'.split()), but some people may find it too "clever."

self.hooks = {self.val("0") : self.out }

Very unclear what hooks is for. This is the only value that is every put in it. self.val("0") seems to be just a complicated way of saying (0,). What's up with that?

def interpret(self, line):
    x = line.split(" ")

Bad variable name: x. Should it be tokens instead?

vals = tuple(self.val(y) for y in x[1:])

Here, vals could be a list instead, like vals = [self.val(y) for y in x[1:]]. Making it a tuple just makes the code a little bit more complicated for no reason.

dereferenced = []
    keys_only = tuple(key for key in self.memory)

Consider keys_only = self.memory.keys()? Oh, and by the way, keys_only is never used, so just remove it.

for val in vals:
      while val in self.memory: val = self.memory[val]

Looks like this while loop will be infinite if it runs at all. Is it supposed to be an if?

dereferenced.append(val)
    vals = tuple(y for y in dereferenced)

Pointless conversion of dereferenced to tuple.

self.dictionary[x[0]](vals)

I think the whole interpret method can be rewritten to something like the following. Note the changed names.

def interpret(self, line):
      command, *tokens = line.split()  # Python 3 only
      values = [self.val(y) for token in tokens]
      dereferenced = [self.memory.get(value, value) for value in values]
      self.dictionary[command](dereferenced)

Some may say it's a bit too terse. I could have written the dereferenced = ... line as a couple of more explicit statements perhaps.

I won't go into the rest, but the comments about naming and unnecessary complexity applies.

StackExchange Code Review Q#2988, answer score: 10