CODEWITHSUNDEEP
pythonpython-3.x
Till
Till

Reputation: 167

python indenting failure?

i was trying to write a class for a little program i've been working on, and i ran into an odd failure of indenting. I was wondering if there's genuinely something wrong with what i wrote, or if it's the parser's fault. This fails in Fedora 15's python 3.2 package.

  def __getitem__(self, key):
    if CharacterStats.usesSubStats:
      if key in self.subStats:
        return self.subStats[key]
    elif key in self.stats: #change this to 'if' and it works
      return self.stats[key]
    else:
      raise KeyError(key)
    #otherwise we end up right here.

As requested so you can run it: http://pastebin.com/d8yQUm3U

Upvotes: 1

Views: 163

Answers (4)

ekhumoro
ekhumoro

Reputation: 120608

Since self.stats and self.subStats are dictionaries, they will both raise a KeyError anyway if they don't contain key.

So why not just write:

def __getitem__(self, key):
    if CharacterStats.usesSubStats:
        return self.subStats[key]
    return self.stats[key]

Or perhaps:

def __getitem__(self, key):
    try:
        if CharacterStats.usesSubStats:
            return self.subStats[key]
        return self.stats[key]
    except KeyError:
        raise CharacterStatsError(key)

Upvotes: 0

DSM
DSM

Reputation: 353059

If I understand you correctly, you want to throw a KeyError if usesSubStats is True and key is not in subStats or if usesSubStats is False and key is not in stats. So I think the problem is that if/elif/else chaining doesn't work like you think it does.

Consider:

def f(x):
    if x == 1:
        return 'first'
    elif x == 2:
        return 'second'
    else:
        return 'other'

produces

>>> f(1), f(2), f(3), f(4)
('first', 'second', 'other', 'other')

which I hope does what's expected and is the pattern you should keep in mind. Since in your test code useSubStats is True, only the first branch is ever tested:

def condition(lab, val):
    print('testing condition', lab);
    return val

def g():
    if condition(1, True):
        return 'first branch'
    elif condition(2, False):
        return 'second branch'
    else:
        return 'other branch'
    return 'fallthrough'


>>> g()
testing condition 1
'first branch'

So your code is behaving like this:

def h():
    if condition(1, True):
        if condition('1b', False):
            return 'first branch'
    elif condition(2, False):
        return 'second branch'
    else:
        return 'other branch'
    return 'fallthrough'

>>> h()
testing condition 1
testing condition 1b
'fallthrough'

I'm not exactly sure how you think it should be behaving, but it seems like you expect that after the "if key in self.subStats" test fails, execution should move back to the next member of the if/elif/else branch one level up and test that. That's simply not how it works, though.

There are several easy ways to get the behaviour you want: one is to flatten it so that it's

    if CharacterStats.usesSubStats and key in self.subStats:

instead, which will evaluate to False and so the next branch will be tested, or --as you discovered yourself -- make the elif an if, which again leads to that condition being independently tested, or rewrite as in an earlier answer.

Does that make sense? The if/elif/else list describes a series of possibilities, with conditions tested sequentially, and only the branch corresponding to the first true condition (taking the final else as 'elif 1:') is executed. You don't move to the next branch depending on what happens within a branch.

Upvotes: 3

NPE
NPE

Reputation: 500357

You can end up on the line marked otherwise we end up right here if and only if CharacterStats.usesSubStats is true and key in self.subStats is false.

When you change the elif to if, you eliminate this possibility: the code can never reach the otherwise we end up right here line.

It's hard to say which of the two versions is correct. Provided I've guessed your intentions correctly, perhaps the following would be a clearer alternative to both of them:

  def __getitem__(self, key):
    if CharacterStats.usesSubStats:
        if key in self.subStats:
            return self.subStats[key]
    elif key in self.stats:
        return self.stats[key]
    raise KeyError(key)

Upvotes: 2

mccc
mccc

Reputation: 2454

I think you should indent the #otherwise.. with the raise, as the latter is inside a final else and the "outer block" will never be reached.. Hence a reasonable, IMO, IndentationError.

Upvotes: 0

Related Questions