On Fri, May 11, 2018 at 8:49 AM, Marko Rauhamaa <ma...@pacujo.net> wrote:
> def split_cmd(self, cmd):
> args = []
> while True:
> match = self.TERM_PTN.match(cmd)
> if match is None:
> return None, None
> args.append(match.group('term'))
> if not match.group('sep'):
> break
> cmd = cmd[match.end(0):]
> verb = args.pop(0).upper()
> return verb, args
>
> You *could* get rid of True. For example:
Yes, you could. For a start, you'd want to add a docstring so someone
else can figure out what on earth is going on here. For instance, I
don't understand why, even after iterating several times, a regex
match failure instantly results in you returning (None, None); is that
an error condition? If so, why is it not raising an exception? What
kind of regex is this (at least, it looks like you're doing regex
work), and can you use re.split() instead of all this? Why are you
uppercasing the verb? Why an ALL_CAPS name (usually a constant) being
referenced from an object (self)? So much of this doesn't look even
slightly Pythonic.
But for the loop itself, you absolutely CAN write this more logically.
I'll take your second version as a template:
def split_cmd(self, cmd):
args = []
while (match := self.TERM_PTN.match(cmd)) is not None:
args.append(match.group('term'))
if not match.group('sep'):
verb = args.pop(0).upper()
return verb, args
cmd = cmd[match.end(0):]
return None, None
And, if this is actually a regex, "is not None" is unnecessary:
while match := self.TERM_PTN.match(cmd):
Now do you understand what I mean about putting the condition into the
loop header?
ChrisA
--
https://mail.python.org/mailman/listinfo/python-list
