1

Here is a bit of code that I'm trying to make more readable. It works, but the nested for loops and the try/if makes it a bit hard to understand at first glance what's going on.

Can someone give me suggestions on how I could possibly join the nested for loops or condense this code?

matcher = None
if re.match(_RE_OBJECT, nodes.replace(LQMN, '')):
  matcher = alias
else:    
  for x in lister[0].conditions:
    for y in x.codes:
      try:
        if y.id.split(',')[1] == condition:
          matcher = x.codenames
      except IndexError:
        pass
nerdinary
  • 67
  • 2
  • 5
  • I don't think you'll make your code "more readable" if you condense your for loops in a single line. Are you looking to get a shorter code, or a more readable code? (Frankly this code is not that unreadable or deeply over-nested...) – ljetibo Mar 05 '15 at 13:00
  • Ideally trying to get more readable code. – nerdinary Mar 05 '15 at 13:01
  • Your `for` loops & `try:... except` block are fine. But you could use `re.compile()` on your `_RE_OBJECT`. (And your code would look more pleasant if you indented with 4 spaces instead of 2. IMHO). ANd if your goal is readibility, then try to come up with more meaningful names than `x` and `y`. Short, throw-away names are ok in single loops, but they start getting a bit obtuse in deeply nested code. – PM 2Ring Mar 05 '15 at 13:04
  • Then putting nested for loops won't help your cause much. I think this is pretty readable, even if I don't know what `lister` is I know what's going on. Additionally the only change I'd make is a deeper indentation. (But that could also be formatting on SO not in actual code, afaik python indent is 4 spaces). – ljetibo Mar 05 '15 at 13:05
  • @ljetibo: Python doesn't care how big your indentation units are, as long as your dedents match your indents. You can use single space indentation - I often do in the interactive interpreter, but it looks _really_ ugly in a script because it makes it really hard to keep track of what's happening. – PM 2Ring Mar 05 '15 at 13:09
  • @PM2Ring I meant, "is 4 spaces by default", so tab produces 4 spaces, you automatically get 4 space indent after a `:` etc...far from it being a constant. – ljetibo Mar 05 '15 at 13:12
  • @ljetibo: That's a common convention in modern editors, but not universal. And it's easily changed. FWIW, Python treats an actual tab character (i.e `\t`, `chr(9)`) in source as equivalent to (up to) **8** spaces, as that's a far older convention. See http://stackoverflow.com/questions/2034517/pythons-interpretation-of-tabs-and-spaces-to-indent and https://docs.python.org/2/reference/lexical_analysis.html#indentation . But I think this discussion is derailing nerdinary's question a bit, so maybe we should continue it in the Python Chat room. – PM 2Ring Mar 05 '15 at 13:17
  • 2 spaces for indentation is a coding standard where I work, so that's not changing. I used x and y here just for asking a question, they have better names :) – nerdinary Mar 05 '15 at 13:30
  • @nerdinary: cool. But you have my sympathy on the 2 space indent thing at work. :) FWIW, I used to do 2 space indents when I first started Python. I've converted most of my old code to 4 spaces, but I occasionally run into old 2 space stuff, and it just looks cramped to me now. – PM 2Ring Mar 05 '15 at 13:34

2 Answers2

5

You could use a generator expression to nest the loops and add a filter that makes the IndexError handler obsolete:

candidates = ((x, y) for x in lister[0].conditions for y in x.codes if ',' in y.id)
for x, y in candidates:
    if y.id.split(',')[1] == condition:
        matcher = x.codenames

Readability would be improved more by using more meaningful names other than x and y here though:

candidates = ((cond, code) for cond in lister[0].conditions for code in cond.codes
              if ',' in code.id)
for cond, code in candidates:
    if code.id.split(',')[1] == condition:
        matcher = cond.codenames
Martijn Pieters
  • 1,048,767
  • 296
  • 4,058
  • 3,343
  • Beat me to the answer. However, since the genexp is essentially *backwards* this is not necessarily more *readable* ... – dhke Mar 05 '15 at 13:03
  • That seems like a clever solution, but python tells me the generator expression has an invalid syntax at the first 'for'. I didn't know that you could actually list a single for after another in one line. – nerdinary Mar 05 '15 at 13:48
  • 1
    @nerdinary: sorry, corrected; the left-hand side expression needs to use parentheses to disambiguate the tuple from the genexp grammar itself. My mistake. – Martijn Pieters Mar 05 '15 at 13:51
1

You can use one-line if and continue statements instead of the try statement:

some_var = y.id.split(',')
if len(some_var) < 2: continue
if some_var[1] == condition:
    matcher = x.codenames

and replace some_var with a meaningful name.

Darrick Herwehe
  • 3,553
  • 1
  • 21
  • 30
TulkinRB
  • 590
  • 1
  • 6
  • 10
  • Exception handling in Python is _very_ efficient, so a proper `try:... except block` is often faster than equivalent `if`-based code. – PM 2Ring Mar 05 '15 at 13:11
  • It always depends on the project you're working on, if speed is not that important, or the code runs fast enough anyway, you'd probably want to use `if`, but if speed is more important you'd use `try:... except`. – TulkinRB Mar 05 '15 at 16:52