2

Como eu faço para melhorar este código?

import random
import string
import requests
from django.core.management.base import BaseCommand, CommandError
from django.core.exceptions import ValidationError
from optparse import make_option
from core.models import Movie


class Command(BaseCommand):
    help = 'Faz o crawler numa api de filmes e retorna os dados.\n \
    Uso: ./manage.py initdata\n \
    ou: ./manage.py initdata -m 20\n \
    ou: ./manage.py initdata -m 20 -y 2015'
    option_list = BaseCommand.option_list + (
        make_option('--movies', '-m',
                    dest='movies',
                    default=10,
                    help='Define a quantidade de filmes a ser inserido.'),
        make_option('--year', '-y',
                    dest='year',
                    default=None,
                    help='Define o ano de lançamento do filme.'),
    )

    def get_html(self, year):
        '''
        Le os dados na api http://www.omdbapi.com/ de forma aleatoria
        e escolhe um filme buscando por 2 letras
        '''
        # escolhe uma letra aleatoriamente
        c = ''.join(random.choice(string.ascii_lowercase)for _ in range(2))
        # se não for definido o ano, então escolhe um randomicamente
        if year is None and isinstance(year, int):
            year = str(random.randint(1950, 2015))
        url = 'http://www.omdbapi.com/?t=' + c + \
            '*&y=' + str(year) + '&plot=short&r=json'
        return requests.get(url).json()

    def save(self, title, year, released, director, actors, poster, imdbRating, imdbVotes, imdbID):
        ''' transforma "imdbVotes" em numero inteiro '''
        if imdbVotes == "'N/A'":
            imdbVotes = 0
        else:
            imdbVotes = imdbVotes.replace(',', '')
        try:
            ''' Salva os dados '''
            Movie.objects.create(
                title=title,
                year=year,
                released=released,
                director=director,
                actors=actors,
                poster=poster,
                imdbRating=imdbRating,
                imdbVotes=imdbVotes,
                imdbID=imdbID,
            )
        except ValidationError:
            print('O objeto não foi salvo.')

    def handle(self, movies, year, **options):
        ''' se "movies" não for nulo, transforma em inteiro '''
        if movies is not None:
            movies = int(movies)
        ''' busca os filmes n vezes, a partir da variavel "movies" '''
        for i in range(movies):
            h = self.get_html(year)
            j = 1  # contador
            ''' Se a resposta for falsa, então busca outro filme '''
            while h['Response'] == 'False' and j < 100:
                h = self.get_html(year)
                print('Tentanto %d vezes' % j)
                j += 1
            ''' se "year" não for nulo, transforma em inteiro '''
            if "–" in h['Year']:
                h['Year'] = year
            # salva
            self.save(h['Title'], h['Year'], h['Released'], h['Director'], h['Actors'],
                      h['Poster'], h['imdbRating'], h['imdbVotes'], h['imdbID'])
            print(i + 1, h['Year'], h['Title'], h['Released'], h['Director'],
                  h['Actors'], h['Poster'], h['imdbRating'], h['imdbVotes'], h['imdbID'])

        # print('Foram salvos %d filmes' % movies)

Por exemplo,

def save(self, title, year, released, director, actors, poster, imdbRating, imdbVotes, imdbID):

Entrar somente com um dicionário de argumentos.

title=title,
year=year,
released=released,
director=director,
actors=actors,
poster=poster,
imdbRating=imdbRating,
imdbVotes=imdbVotes,
imdbID=imdbID,

Será que dá pra transforma em algo menor, um dicionário talvez.

self.save(h['Title'], h['Year'], h['Released'], h['Director'], h['Actors'],
          h['Poster'], h['imdbRating'], h['imdbVotes'], h['imdbID'])

Também poderia ser otimizado?

5
  • Recomendo fortemente a leitura do "Código Limpo": buscape.com.br/codigo-limpo-robert-cecil-martin-8576082675.html . E evite usar "try except" em seus códigos, ainda mais se você vai apenas dizer: "print('O objeto não foi salvo.')". Você coloca um try except porque espera um erro. Qual o sentido de você fazer um pedaço de código que te gere um erro? O Zen do Python diz: "erros não devem ser silenciados, a menos que sejam explicitamente silenciados". Se você tem um except, você deve de alguma forma receber um email de traceback ou alguma notificação de que tal problema aconteceu.
    – Puam Dias
    Commented 23/11/2015 às 12:12
  • 1
    Vale a pena tu procurar por bulk_create também. Em vez de fazer 100 Movie.objects.create(), você faria 1 Movie.objects.bulk_create(lista). Iria otimizar esse processo ai. docs.djangoproject.com/en/1.8/ref/models/querysets/#bulk-create
    – Puam Dias
    Commented 23/11/2015 às 12:35
  • @PuamDias Concordo com quase tudo o que você diz, exceto nessa questão do try...except. O ideal seria informar a razão do erro, claro, mas nesse caso está óbvio que é um erro de validação, e se a intenção do AP for que o programa não pare após encontrar um dado inválido (i.e. que ele ignore o inválido e continue processando os demais) então não há muita saída a não ser usar try...except. Da mesma forma, bulk_create seria o ideal num cenário de "tudo ou nada", mas da mesma forma não daria pra usar (acho) caso se quisesse processar os válidos e ignorar os inválidos.
    – mgibsonbr
    Commented 23/11/2015 às 13:25
  • 1
    @mgibsonbr, o que quis dizer nessa parte do try except, é que muita gente acha "normal" colocar isso em qualquer parte do código. Nesse caso dele é justo ter um try except porque ele está importando algo de agente externo a aplicação dele (pelo que parece né), mas ele precisa ter uma notificação do motivo do traceback. Se os dados importados forem dele mesmo, eu acho "sem sentido" desenvolver algo que eu espere erro daquilo e tenha que colocar try except no código.
    – Puam Dias
    Commented 24/11/2015 às 12:32
  • Eu coloquei tryporque não tratei todos os erros provenientes dos dados da api. Commented 25/11/2015 às 18:05

1 Resposta 1

3

Se o problema for a entrada de argumentos, basta usar o operador **, sem mexer em nada no seu código:

h = {
    "title":"foo",
    "year":2015,
    ...
    "imdbID":12345,
}

self.save(**h)
# é o mesmo que
self.save(h["title"], h["year"], ..., h["imdbID"])

Para você usar dessa forma, é importante que o dicionário h possua o mesmo número e nomes de todos os parâmetros. Se você quiser que alguns parâmetros sejam opcionais, pode declará-los com um valor padrão, e assim a ausência dele não trará problemas:

def save(self, title, year, released=False, director, actors, poster, imdbRating, imdbVotes, imdbID):
    ...

self.save(**h) # Mesmo que o anterior
del h["released"]
self.save(**h) # Ainda funciona; released é recebido como False

De forma conversa, se você quer simplificar o recebimento dos parâmetros, e ainda permitir que a função seja chamada com argumentos posicionais, você pode usar os operadores * e ** na definição da função (usualmente chamados de *args e **kwargs). Mas isso você já sabe como usar, pelo que vejo já tem isso no seu código.

Por fim, dá pra usar ambos: recebe os parâmetros num dicionário, usando **, depois passa esse dicionário para Movie.objects.create também usando **:

def save(self, **kwargs):
    ''' transforma "imdbVotes" em numero inteiro '''
    if kwargs["imdbVotes"] == "'N/A'":
        kwargs["imdbVotes"] = 0
    else:
        kwargs["imdbVotes"] = kwargs["imdbVotes"].replace(',', '')
    try:
        ''' Salva os dados '''
        Movie.objects.create(**kwargs)

E no handle:

# salva
self.save(**h)

P.S. Se os parâmetros da sua função são posicionais e nomeados, tal como no seu exemplo, também dá pra chamá-los usando uma lista ou tupla:

h = ("foo", 2015, ..., 12345)
self.save(*h) # title recebe "foo", year recebe 2015, etc

Você deve fazer log-in para responder a esta pergunta.

Esta não é a resposta que você está procurando? Pesquise outras perguntas com a tag .