Review python program

Iniciado por sergitronico, Agosto 11, 2016, 02:04:48 PM

Tema anterior - Siguiente tema

0 Miembros y 1 Visitante están viendo este tema.

Agosto 11, 2016, 02:04:48 PM Ultima modificación: Agosto 11, 2016, 02:07:14 PM por Gabriela
I have a python code named No tienes permitido ver los links. Registrarse o Entrar a mi cuenta which uses inputs to open different web pages in which there are links to download pokémon episodes. My question is 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. The program has a module called No tienes permitido ver los links. Registrarse o Entrar a mi cuenta 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 like this: P0k3M0N.1x01.Es.avi.mp4 thanks. Principal code:

Código: python

    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:
Código: python

    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:
83
34
40
51
64
39
51
53
46
51
51
52
33
49
48
44
47
44

Thanks. Waiting answers.

Código: php

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


you are reading too many files. For example, the one with the code will be appended to the list

Código: php

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

I have no idea why you use two txt files. Both have the same name, so either you just need one or the names aren't descriptive enough. As a last option, add a comment

Why f1 and f4? what happened to f2 and f3?

Also, calling variables by it's time is wrong. I know str1 is a string by seeing what is being assigned to it, but i want to know what it's used for. The same goes for list1, list2, etc

Código: php

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

Again, you are doing the same with two different variables. I can't tell the difference between "Last:" and "Last one:"

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

i should be used only in for i in xrange(...
everybody assumes it's an integer, not anything else.

assuming I understood it all, this is way more clear:
Código: php
for file in files:
    index = file.find("x")
   episode_name = i[index + 1:index + 3]
    episodes.append(episode_name)


You shpuld declare out just before the while where it's used, it makes it easier to quickly see what it's for

related to that, you should use functions to divide your problem in small portions with proper names

using xrange is more efficient than range without any downsides

instead of having the urls like that, you could declare them all in the same place as constants and replace the variable parts with this No tienes permitido ver los links. Registrarse o Entrar a mi cuenta
python doesn't have real constants but, by convention, any variable name in uppercase meants it shouldn't be changed


Try to fix all that, post your code and I will review it again