On 19 March 2012 23:20, Ian Kelly <ian.g.ke...@gmail.com> wrote: > I hope you don't mind if I critique your code a bit! > > On Fri, Mar 16, 2012 at 7:21 PM, Kiuhnm > <kiuhnm03.4t.yahoo...@mail.python.org> wrote: >> Here we go. >> >> ---> >> def genCur(f, unique = True, minArgs = -1): > > It is customary in Python for unsupplied arguments with no default to > use the value None, not -1. That's what it exists for. > >> """ Generates a 'curried' version of a function. """ >> def geng(curArgs, curKwargs): >> def g(*args, **kwargs): >> nonlocal f, curArgs, curKwargs, minArgs; # our STATIC data
I don't know if all the rest of the code is below, but this line above would only be necessary if you want to rebind f, curArgs, minArgs. You don't seem to do it, so I think this line is unnecessary. Also, your naming of variables disagrees with PEP 8 :) >> if len(args) or len(kwargs): > > Collections evaluate as true if they are not empty, so this could just be: > > if args or kwargs: > >> # Allocates data for the next 'g'. We don't want to modify our >> # static data. >> newArgs = curArgs[:]; Semicolon to end a statement? >> newKwargs = dict.copy(curKwargs); >> >> # Adds positional arguments. >> newArgs += args; >> >> # Adds/updates keyword arguments. >> if unique: >> # We don't want repeated keyword arguments. >> for k in kwargs.keys(): >> if k in newKwargs: >> raise(Exception("Repeated kw arg while unique = >> True")); >> newKwargs.update(kwargs); > > Since you're writing this for Python 3 (as evidenced by the use of the > nonlocal keyword), you could take advantage here of the fact that > Python 3 dictionary views behave like sets. Also, you should use a > more specific exception type: > > if unique and not kwargs.keys().isdisjoint(newKwargs): > raise ValueError("A repeated keyword argument was > supplied") > >> # Checks whether it's time to evaluate f. >> if minArgs >= 0 and minArgs <= len(newArgs) + len(newKwargs): > > With minArgs defaulting to None, that would be: > > if minArgs is not None and minArgs <= len(newArgs) + > len(newKwargs): > >> return f(*newArgs, **newKwargs); # f has enough args >> else: >> return geng(newArgs, newKwargs); # f needs some more >> args >> else: >> return f(*curArgs, **curKwargs); # the caller forced the >> evaluation >> return g; >> return geng([], {}); >> >> def cur(f, minArgs = -1): >> return genCur(f, True, minArgs); >> >> def curr(f, minArgs = -1): >> return genCur(f, False, minArgs); > > The names "cur" and "curr" are terrible. Good names should describe > what the function does without being too onerous to type, and the > addition of the duplicate "r" is not an obvious mnemonic for > remembering that the first one prohibits duplicate keyword arguments > and the second one allows them. Why not more descriptive names like > "curry" and "curry_unique"? > > That's all I've got. All in all, it's pretty decent for a Python newbie. > > Cheers, > Ian > -- > http://mail.python.org/mailman/listinfo/python-list -- http://mail.python.org/mailman/listinfo/python-list