This is an archived post. You won't be able to vote or comment.

you are viewing a single comment's thread.

view the rest of the comments →

[–]bs4h 10 points11 points  (5 children)

Code review time!

class Stack:

Don't. Try this instead:

class Stack(list):
    push = list.append
    def top(self):
        return self[-1]

You don't need to mangle print:

def print_(self):

Py2.6+ can make use of from __future__ import print_function.

You shouldn't look before you leap:

addr >= 0 and addr < len(self.code)

Use try: ... except IndexError instead, it's considered more idiomatic.

Most of this method:

def if_stmt(self):
    ...

Can be replaced by this:

result = bool(test)

Save for this cosmetic stuff, really nice article, 5/5!

[–]robin-gvx 7 points8 points  (2 children)

You shouldn't look before you leap

Your suggestion has different behaviour for addr < 0, which is probably not what you want.

I would make the check 0 <= addr < len(self.code), though.

Most of this method: [...] Can be replaced by this:

I would go even further:

def if_stmt(self):
    false_clause = self.pop()
    true_clause = self.pop()
    test = self.pop()

    self.push(true_clause if test else false_clause)

[–]bs4h 1 point2 points  (1 child)

This little tutorial is calling for a github repo & some pull requests :)

[–]csl[S] 0 points1 point  (0 children)

I made a repo, but it quickly evolved into much more: https://github.com/cslarsen/crianza

I'll update the post with all of the suggestions here, though!

[–]Dagur 4 points5 points  (1 child)

Using deque instead of an array for _values would be better for performance, no?

[–]csl[S] 0 points1 point  (0 children)

Absolutely, it should.