Note that this expression actually does nothing (it's the same as yours, except for a couple of corrections, but spread over a few lines for readability):
map(lambda k,v: self._value_store.update((k,v)) # k=v always sets key 'k'
if self.keyword_check(k) == True
else print('bad parameter'),
kwargs.keys(),
kwargs.values()
)
map
returns a generator. Only when you collect the values of the generator will it actually execute the side effect (self._value_store.update
). So to get it to actually update the object's internal value store, you'd need to do something like
list(map(lambda k,v: self._value_store.update((k,v))
if self.keyword_check(k) == True
else print('bad parameter'),
kwargs.keys(),
kwargs.values()
))
(You could replace list
with all
to avoid creating a useless list of None
s; all
will work here because {}.update()
always returns None
, but that's probably even worse programming style.)
In general, using map
only to produce a side effect is not usually good style, unless you really have a need for the execution of the side effects to be delayed (and in that case, some documentation is called for).
Instead, pass the generator to update
as its first argument; update
iterates over its first argument, so the generator will be fully evaluated. Also, the expression is somewhat simpler and (I think) easier to read:
self._value_store.update((k,v) for k,v in kwargs.items()
if self.keyword_check(k))
You might find an explicit loop is even more readable:
for k, v in kwargs.items():
if self.keyword_check(k):
self._value_store[k] = v
# Or, possibly faster (or possibly not):
for k in kwargs.keys():
if self.keyword_check(k):
self._value_store[k] = kwargs[k]
By the way, testing the value returned by a boolean function for equality with True
is also an anti-pattern (which is why I didn't do it above). It risks ignoring a value which passed the test, if the predicate might return a truthy value other than True
.