Sending users to web pages to download Pokemon episodes

Question

I have a python code named seasons.py which uses inputs to open different web pages in which there are links to download Pokémon episodes.

The program has a module called seasons.py where it returns the number of episodes that has the season which was entered. It also needs a number of text files called last.txt, and seasons.txt, last2.txt.

The Pokémon episodes names are formatted like:

P0k3M0N.1x01.Es.avi.mp4

How can I improve the code and prevent errors? If you could tell me better names for the variables it will be perfect, but if not, it's OK.

import webbrowser
import os
import seasons
import time

f1 = open('last.txt', 'r')
f4 = open('last2.txt', 'r')

out = False
list1 = []
list2 = []

for file in os.listdir("."):
    if "P0k3M0N" in file:
        list1.append(file)

for i in list1:
    index = i.find("x")
    str1 = i[index + 1:index + 3]
    list2.append(str1)

print("Last one: " + f1.readline())
print("Last: " + f4.readline())

f1.close()
f4.close()

x = 1
list3 = []
list4 = []

for i in range(int(list2[-1])):
    y = str(x)
    if y in list2:
        list3.append(y)
    x += 1

count1 = 0
count2 = 0

for i in list4:
    count1 += 1

for i in list3:
    count2 += 1

if count1 != 0:
    print("You have: " + str(list4))

if count2 != 0:
    print("You have consecutives from " + list3[0] + " to " + list3[-1])


while out == False:
    print("[1] Full season")
    print("[2] Single episodes: Examples: 1-10 / 1, 2 / 3")
    print("[3] Get out")
    input2 = input()
    input1 = str(input2)
    if input1 == "3":
        out = True
    else:
        if input1 == "1":
            season = input("Season: ")
            episodes = seasons.episodes(season)
            for i in range(episodes):
                if len(str(i + 1)) == 1:
                    url = "http://seriesblanco.com/serie/979/temporada-" + str(season) + "/capitulo-0" + str(x) + "/pokemon.html"
                    webbrowser.open_new_tab(url)
                    time.sleep(20)
                else:
                    url = "http://seriesblanco.com/serie/979/temporada-" + str(season) + "/capitulo-" + str(x) + "/pokemon.html"
                    webbrowser.open_new_tab(url)
                    time.sleep(20)
        else:
            input4 = input("Season: ")
            season = str(input4)
            file = open('season.txt', 'r')
            total_eps = file.readlines()
            number = total_eps[int(input4) - 1]
            print("This season has " + number + " episodes")
            input3 = input("Episodes: ")
            episodes = str(input3)
            if "-" in episodes:
                index = episodes.find("-")
                first = episodes[:index]
                second = episodes[index + 1:]
                x = int(first)
                numbers = "123456789"
                for i in range(int(second) - int(first) + 1):
                    if str(x) in numbers:
                        url = "http://seriesblanco.com/serie/979/temporada-" + season + "/capitulo-0" + str(x) + "/pokemon.html"
                        webbrowser.open_new_tab(url)
                    else:
                        url = "http://seriesblanco.com/serie/979/temporada-" + season + "/capitulo-" + str(x) + "/pokemon.html"
                        webbrowser.open_new_tab(url)
                    x += 1
                    if int(second) > 10:
                        time.sleep(10)
                os.remove("last2.txt")
                f3 = open('last2.txt', 'w')
                f3.write(episodes)
                f3.close()
            elif "," in episodes:
                numbers = "123456789"
                count = 0
                list = []
                index = episodes.find(",")
                number = episodes[:index]
                list.append(number)
                while index != -1:
                    index = episodes.find(",")
                    if episodes[index + 1] == " ":
                        episodes = episodes[index + 2:]
                    elif episodes[index + 1] in numbers:
                        episodes = episodes[index + 1:]
                    number = episodes[:index]
                    list.append(number)
                list.pop(-1)
                for i in list:
                    count += 1
                for i in range(count):
                    if len(list[i]) == 1:
                        url = "http://seriesblanco.com/serie/979/temporada-" + season + "/capitulo-0" + list[i] + "/pokemon.html"
                        webbrowser.open_new_tab(url)
                    else:
                        url = "http://seriesblanco.com/serie/979/temporada-" + season + "/capitulo-" + list[i] + "/pokemon.html"
                        webbrowser.open_new_tab(url)
            else:
                if len(episodes) == 1:
                    url = "http://seriesblanco.com/serie/979/temporada-" + season + "/capitulo-0" + episodes + "/pokemon.html"
                    webbrowser.open_new_tab(url)
                    os.remove("last.txt")
                    f2 = open('last.txt', 'w')
                    f2.write(episodes)
                    f2.close()
                else:
                    url = "http://seriesblanco.com/serie/979/temporada-" + season + "/capitulo-" + episodes + "/pokemon.html"
                    webbrowser.open_new_tab(url)
                    os.remove("last.txt")
                    f2 = open('last.txt', 'w')
                    f2.write(episodes)
                    f2.close()

The other module is here:

def episodes(n_season):
    f = open('season.txt', 'r')
    seasons = f.readlines()
    season = seasons[int(n_season) - 1]
    index = season.find('\n')
    return season[:index]

seasons.txt contains this:

83 34 40 51 64 39 51 53 46 51 51 52 33 49 48 44 47 44

Answer 1

You can save a lot of code duplication by using format:

if len(episodes) == 1:
    url = "http://seriesblanco.com/serie/979/temporada-" + season + "/capitulo-0" + episodes + "/pokemon.html"
    webbrowser.open_new_tab(url)
    os.remove("last.txt")
    f2 = open('last.txt', 'w')
    f2.write(episodes)
    f2.close()
else:
    url = "http://seriesblanco.com/serie/979/temporada-" + season + "/capitulo-" + episodes + "/pokemon.html"
    webbrowser.open_new_tab(url)
    os.remove("last.txt")
    f2 = open('last.txt', 'w')
    f2.write(episodes)
    f2.close()

Becomes (with the with..as construct as well):

url = "http://seriesblanco.com/serie/979/temporada-{}/capitulo-{:02d}/pokemon.html"
webbrowser.open_new_tab(url.format(season, episode))
with open('last.txt', 'w') as f2:
    f2.write(episodes)

Here, the {:02d} makes sure that the number has two digits and will fill with 0 if it has not. This also takes away all the code with numbers=... to check whether the number is one or two digits. The format syntax also happily takes in any type as arguments and calls str() on them, so this works:

>>> "{:02d}".format(1)
'01'
>>> "{:02d}".format(10)
'10'

Also note that when specifying the 'w' flag, the file will be overwritten, so there is no need to delete it first.

The building of the url and the opening in a webbrowser is ripe to be outsourced to a function:

def open_in_new_tab(webbrowser, season, episode):
    url = "http://seriesblanco.com/serie/979/temporada-{}/capitulo-{:02d}/pokemon.html"
    webbrowser.open_new_tab(url.format(season, episode))

Answer 2

f1 = open('last.txt', 'r')
f4 = open('last2.txt', 'r')

out = False
list1 = []
list2 = []

for file in os.listdir("."):
    if "P0k3M0N" in file:
        list1.append(file)

for i in list1:
    index = i.find("x")
    str1 = i[index + 1:index + 3]
    list2.append(str1)

print("Last one: " + f1.readline())
print("Last: " + f4.readline())

f1.close()
f4.close()

I would avoid dealing with files in this way. If there is an error anywhere in between the first two and the last two lines shown above, your code will stop and the file will be left open.

It's better practice to use the with statement - that way, if there are any errors, the file will still be closed.

with open('last.txt', 'r') as f:
    print("Last one: " + f.readline())

with open('last2.txt', 'r') as f:
    print("Last: " + f.readline())

Answer 3

• In general the string handling is a bit more complex than necessary, e.g. in episodes what you want to do is get the "nth" episode from the file, without the newline, so it's better for the reader if you just write it that way. There's no reason to have the int conversion in there either, that should happen at an outer level while parsing user input if at all. In general it would probably also be better for reusability to pass in the filename instead of hardcoding it. E.g. that could look at least like this now:

  def episodes(n_season):
      with open('season.txt', 'r') as f:
          return f.readlines()[n_season - 1].strip()
  

• I'd generally put variables close to where they are used - declarations at the top make it just harder to find things (e.g. out).

• list1 can be inlined and condensed quite a bit, e.g.:

  for file in os.listdir("."):
      if "P0k3M0N" in file:
          index = file.find("x")
          str1 = file[index + 1:index + 3]
          list2.append(str1)
  

  Then again, the string operation here is better and more readabily written with a regular expression, e.g.:

  list2 = [re.search("(\d)x(\d+)", file).groups()[1]
          for file in os.listdir(".")
          if "P0k3M0N" in file]
  

  Also note that by using file it's shadowing a standard library function, which is arguably bad (since you might forget about it and later find out when you try to use the standard file function instead).

  This regex matches a bit more than what the current version did, but again that doesn't seem particularly bad, while at the same time it's much easier to get what it does.

  list2 is still a bad name of course. Even just files would be better.

• x and it's usage are relatively obscure. I'm hesitant to suggest changes in the block that creates list3 as I'm relatively sure that it doesn't quite work ... if y in files means that if files contains strings like 01 or 10, and y = str(x) means that y is e.g. 1, then the test should fail. Perhaps store only the integers in files then?
• list4 is half unused, i.e. it'll always be empty.
• The count1 / count2 variables can simply be len(list4) / len(list3), no need for any more logic. The message "You have consecutives..." doesn't seem to hold up given the logic of the code though. There are no guarantees that you have all elements consecutive in list3.

Dat loop. You might want to start putting things into functions etc. to make things more manageable. If you find yourself writing the same thing again with barely any difference you should also try to find a way to reconcile - there are about four duplicate blocks here that manipulate URLs - those should all be unified.

• ... == False is better written as not ....
• else: if ...: can also be written with one level less nesting as elif ...:.
• Having a zero prefix for integer formatting is better accomplished by formatting directives, e.g. "{:02d}".format(1).
• Put the URL(s) as constants at the top. You can much easier change them if they're in one place. In the future you might also want to have them overridable as command-line arguments and again it's much easier to make those changes if you're already using a single variable for it.

Once you have all the duplication cleaned up we can look at more improvements, but at this point there's just a bit too much going on.