2
for tag in tags:
    Ulist1.append(tag.get('href', None))

if len(Ulist1) > 2:
    print Ulist1[2]
    html = urllib.urlopen(Ulist1[2]).read()
    soup = BeautifulSoup(html)
    tags = soup('a')
    Ulist2 = list ()

    for tag in tags:
        Ulist2.append(tag.get('href', None))

    if len(Ulist2) > 2:
        print Ulist2[2]
        html = urllib.urlopen(Ulist2[2]).read()
        soup = BeautifulSoup(html)
        tags = soup('a')
        Ulist3 = list ()

        for tag in tags:
            Ulist3.append(tag.get('href', None))

        if len(Ulist3) > 2:
            print Ulist3[2]
            html = urllib.urlopen(Ulist3[2]).read()
            soup = BeautifulSoup(html)
            tags = soup('a')
            Ulist4 = list ()

            for tag in tags:
                Ulist4.append(tag.get('href', None))

this is using beautiful soup to parse HTML and find the link at position 3 (the first name is 1). Follow that link. Repeat this process 4 times. Is there a more efficient way of doing this instead of using nested loops?

2
  • 5
    If you have working code that you think could be improved, see Code Review
    – jonrsharpe
    Jan 8, 2017 at 23:47
  • 1
    If you copy and paste code, and then make slight changes, you should probably factor it into a function, and pass in the differences as parameters.
    – Peter Wood
    Jan 8, 2017 at 23:52

2 Answers 2

2

You could factor it out into a function as Peter Wood said. Here's one possible implementation which shows the basic concept.

def print_third_recursive(tags, iterations):
    Ulist = [tag.get('href', None) for tag in tags] # more pythonic
    if len(Ulist) > 2 && iterations :
        print Ulist[2]
        html = urllib.urlopen(Ulist[2]).read()
        soup = BeautifulSoup(html)
        new_tags = soup('a')
        use_third(new_tags, iterations - 1)

use_third_recursive(tags, 3)

This could definitely be done without using recursion as well if you want the function to be simpler.

def print_third(tags):
    Ulist = [tag.get('href', None) for tag in tags] # more pythonic
    new_tags = []
    if len(Ulist) > 2:
        print Ulist[2]
        html = urllib.urlopen(Ulist[2]).read()
        soup = BeautifulSoup(html)
        new_tags = soup('a')
    return new_tags

print_third(
    print_third(
        print_third(tags)
    )
)

Neither implementation should have any issues if there are not 3 items in one of the tag lists as they'll just return right back out of the layers.

0

As Peter and Anthony said, you can extract method and make things simpler.

But, a thumb rule in general, instead of nested ifs, you can change the condition to its complement and return.

in your example:

if len(Ulist1) > 2: do_stuff() if len(Ulist1) > 2: do_more_stuff()

instead you can write it as follows:

if len(Ulist1) < 2: # the compement of the original condition return do_stuff() if len(Ulist1) < 2: # the compement of the original condition return do_more_stuff()

So, your code can be written as follows:

if len(Ulist1) < 2:
    return

print Ulist1[2]
html = urllib.urlopen(Ulist1[2]).read()
soup = BeautifulSoup(html)
tags = soup('a')
Ulist2 = list ()

for tag in tags:
    Ulist2.append(tag.get('href', None))

if len(Ulist2) < 2:
    return

print Ulist2[2]
html = urllib.urlopen(Ulist2[2]).read()
soup = BeautifulSoup(html)
tags = soup('a')
Ulist3 = list ()

for tag in tags:
    Ulist3.append(tag.get('href', None))

if len(Ulist3) < 2:
    return

print Ulist3[2]
html = urllib.urlopen(Ulist3[2]).read()
soup = BeautifulSoup(html)
tags = soup('a')
Ulist4 = list ()

for tag in tags:
    Ulist4.append(tag.get('href', None))

And, of course, I suggest you extract the method as Anthony wrote above.

Hope it helps.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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