current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 9 down vote favorite

I have the following code, written in Python:

if id:
    fields = r.table(cls._tablename)
              .get(id)
              .run(cls.connection())
else:
    try:
        fields = list(r.table(cls._tablename)
                       .filter(**kwargs)
                       .limit(1)
                       .run(cls.connection()))[0]
    except IndexError:
        fields = None

The code acts as a wrapper for getting a document from a RethinkDB datastore. I find all those indents unaesthetic, and I cannot seem to find a way to wrap the lines to look nicer.

So, is there a way I can rewrite (or maybe just rewrap) this code to look better and provide the same functionality?

share|improve this question
add comment

4 Answers

up vote 3 down vote accepted

I'd do something like

table = r.table(cls._tablename)

if id:
    fields = table.get(id).run(cls.connection())

else:        
    filtered = table.filter(**kwargs).limit(1)
    fieldss = filtered.run(cls.connection()) 

    # Consider "next(iter(fieldss), None)"
    try:
        fields = list(fieldss)[0]        
    except IndexError:
        fields = None

where I replace tableid, table, filtered and fieldss with descriptive names (rather than my generic stand-ins).

I find that typically there's no reason to try and be too clever with the workflow. It's easier to have logically discrete steps; a chain might look pretty but it's harder to comment, format, move and reason about. For example, the next tip in the code was only obvious once I split up the lines.

share|improve this answer
    
You are right, I tended to put everything in one command, but it looks prettier if I split everything into smaller, readable parts. Thank you! –  Andrei Horak yesterday
add comment
up vote 5 down vote

Your indentations strike me as reasonable. In any case, in functions you can often use return statements to help avoid indentation ...

def get_fields(id):
    if id:
        fields = ...
        return fields

    try:
        fields = ...
    except:
        fields = ...

    return fields
share|improve this answer
add comment
up vote 2 down vote

PEP8 allows for violations when it makes sense (and it does in this case. For instance, you can try something like this:

if id:
    fields = r.table(cls._tablename).get(id).run(cls.connection())
else:
    try:
        fields = list(
            r.table(cls._tablename)
                .filter(**kwargs)
                .limit(1)
                .run(cls.connection())
        )[0]
    except IndexError:
        fields = None

Or this:

if id:
    fields = r.table(cls._tablename)
        .get(id)
        .run(cls.connection())
else:
    try:
        fields = list(r.table(cls._tablename)
            .filter(**kwargs)
            .limit(1)
            .run(cls.connection()))[0]
    except IndexError:
        fields = None

Whatever you think looks best in your situation, even if it violates PEP8.

share|improve this answer
add comment
up vote 1 down vote

Not really much you can do about the aesthetics of Python's indent style. Perhaps you can do as such:

if id:
    fields = r.table(cls._tablename).get(id).run(cls.connection())
else:
    try:
        fields = list(r.table(cls._tablename).filter(**kwargs).limit(1).run(cls.connection()))[0]
    except IndexError:
        fields = None

of maybe break the line with a backslash ():

if id:
    fields = r.table(cls._tablename).get(id).run(cls.connection())
else:
    try:
        fields = \
            list(r.table(cls._tablename).filter(**kwargs).limit(1).run(cls.connection()))[0]
    except IndexError:
        fields = None
share|improve this answer
5  
The lines become too long this way and don't comply to PEP8 anymore. –  Andrei Horak yesterday
1  
Hmm, good point –  jsanc623 yesterday
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.