9
\$\begingroup\$

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?

\$\endgroup\$

closed as off-topic by Graipher, Stephen Rauch, Toby Speight, t3chb0t, Sᴀᴍ Onᴇᴌᴀ Jul 12 '18 at 13:00

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." – Graipher, Stephen Rauch, Toby Speight, t3chb0t, Sᴀᴍ Onᴇᴌᴀ
If this question can be reworded to fit the rules in the help center, please edit the question.

4
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ 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! \$\endgroup\$ – linkyndy Apr 16 '14 at 8:01
5
\$\begingroup\$

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
\$\endgroup\$
2
\$\begingroup\$

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.

\$\endgroup\$
1
\$\begingroup\$

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
\$\endgroup\$
  • 5
    \$\begingroup\$ The lines become too long this way and don't comply to PEP8 anymore. \$\endgroup\$ – linkyndy Apr 15 '14 at 19:53
  • 1
    \$\begingroup\$ Hmm, good point \$\endgroup\$ – jsanc623 Apr 16 '14 at 0:11

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