espeed / bulbs

A Python persistence framework for graph databases like Neo4j, OrientDB and Titan.
http://bulbflow.org
Other
622 stars 83 forks source link

Return an empty list instead of None on server requests. #75

Closed barbogast closed 11 years ago

barbogast commented 11 years ago

I would like it if server requests like in the example below would return an empty list (or iterator) instead of None if there are no elements. This way I could iterate over the result without checking if the result is None.

My current code:

edges_iter = element.inE('belongs_to')
if edges_iter:  # check if the result is not None
    for edge in edges_iter:
        pass

How it would look like:

for edge in element.inE('belongs_to'):
    pass

Because of the different elements in the tree my code tends to get deeply nested. With this change I can ommit one level of indentation for every level in the tree.

My current solution:

def _NTL(v):
    """ NoneToList: Convert None into [] """
    return [] if v is None else v

for edge in _NTL(element.inE('belongs_to')):
    pass
kefirbandi commented 11 years ago

+1! I struggle with the same issue

kefirbandi commented 11 years ago

Looking at the code, this seems to be a design decision, achieved by the line

if response.total_size > 0:

in initialize_elements function of the utils.py file. Removing this line solves the problem, but I assume it is not there without a reason.

espeed commented 11 years ago

Some consider it bad Python style to return empty iterators rather than None.

However, you can get rid of the if statement by doing:

edges = element.inE('belongs_to') or []

...or...

for edge in element.inE('belongs_to') or []:
    pass
espeed commented 11 years ago

You can reduce the code even further by using a list comprehension or a generator expression:

[some_func(edge) for edge in element.inE('belongs_to') or []]

See...

barbogast commented 11 years ago

Thanks for your response. I like the idea of doing the check for None inline, that shouldn't add too much boilerplate.

I can see the downside when returning an empty iterator. If someone wants to only check for an empty iterator he would have to do something like this

if list(my_iter): 
    pass

which could be very expensive if the result of the iterator is big.

Or he could to

try:
    my_iter.next()
except StopIteration:
    pass

which is a lot more code than to check if it is None.

kefirbandi commented 11 years ago

You are right guys, returning empty list/generator is not always a better option than returning None. However in most cases I prefer [] to None, so I do the following: Most of the cases when I do a server request I do it through a function which is a member of my model class, like this:

class C(Node):
    ...
    def children(self):
        return self.outV('SubComponent')

I wrote a decorator which gives me the option to turn None to [] or leave it as is, with the conversion being the default case:

def NoneConverter(f):
    """ Decorator to enhance a function f, with the option to convert None to [] (default) or leave as it is. """
    def mf(self,convert = True):
        if convert:
            return f(self) or []
        else:
            return f(self)
    return mf

So now I write:

class C(Node):
    ...
    @NoneConverter
    def children(self):
        return self.outV('SubComponent')

if I write >>> C_type_object.children() it gets converted, but if I don't want it to, I write >>> C_type_object.children(False)

espeed commented 11 years ago

Resolved.