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.

Are there ways to avoid this triply nested for-loop?

def add_random_fields():
from numpy.random import rand
server = couchdb.Server()
databases = [database for database in server if not database.startswith('_')]
for database in databases:
    for document in couchdb_pager(server[database]):
        if 'results' in server[database][document]:
            for tweet in server[database][document]['results']:
                if tweet and 'rand_num' not in tweet:
                    print document
                    tweet['rand_num'] = rand()
                    server[database].save(tweet)

Update

People suggesting that I use a generator reminded me that I forgot to mention that couchdb_pager is a generator over the list server[database].

share|improve this question
4  
Fundamentally, you're descending through three separate levels of data. Unless you can flatten them or skip one, you have to use a for-loop for each level in the hierarchy. I'd suggest refactoring some of your loops out into their own functions. –  Twisol Dec 16 '12 at 5:37
    
Maybe using a generator or iterator? –  DrewV Dec 16 '12 at 6:48
3  
As it is I don't see any problem with this code - it's readable and efficient. If you really don't like the nesting you could do something like documents = (document for database in databases for document in couchdb_pager(server[database]) if 'results' in document) then loop through the documents. –  Stuart Dec 16 '12 at 7:46
add comment

2 Answers

There are a few tweaks that can improve this code:

def add_random_fields():
    from numpy.random import rand
    server = couchdb.Server()
    databases = (server[database] for database in server if not database.startswith('_'))
    # immediately fetch the Server object rather then the key, 
    # changes to a generator to avoid instantiating all the database objects at once
    for database in databases:
        for document in couchdb_pager(database):
            document = database[document] 
            # same here, avoid holding keys when you can have the objects

            for tweet in document.get('results', []):
            # rather then checking for the key, have it return a default that does the same thing
                if tweet and 'rand_num' not in tweet:
                    print document
                    tweet['rand_num'] = rand()
                    database.save(tweet)
share|improve this answer
add comment

I agree with the question comments - there's no real way to avoid them (unless you can restructure the databases!), but the code can be made more readable by removing the loops into a separate generator function, especially since you're not doing any processing between each loop.

Consider something like (caution: untested):

def find_all_document_tweets( server ):
    databases = [database for database in server if not database.startswith('_')]
    for database in databases:
        for document in couchdb_pager(server[database]):
            if 'results' in server[database][document]:
                for tweet in server[database][document]['results']:
                    yield database, document, tweet

def add_random_fields():
    from numpy.random import rand
    server = couchdb.Server()
    for ( database, document, tweet ) in find_all_document_tweets( server ):
        if tweet and 'rand_num' not in tweet:
            print document
            tweet['rand_num'] = rand()
            server[database].save(tweet)

As an extra bonus, it seems likely that the loops now in find_all_document_tweets will be common in your code, so that it's now usable elsewhere.

share|improve this answer
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.