yuriscosta-zz / Weather-Bot

This Messenger bot shows some information about the current weather in the user shared location
https://www.facebook.com/weather.info.bot
MIT License
4 stars 0 forks source link

Seria interessante refatorar alguns "bad smells" #4

Open Mazuh opened 7 years ago

Mazuh commented 7 years ago

Em Python, indentação é muito importante. O código perde consideravelmente a portabilidade quando usa tabs ("/t") ao invés de quatro espaços (" "). Apesar de que, até o Python 2, o interpretador aceita misturar ambas as indentações, vai contra as convenções. Na maioria dos IDEs e editores de gente o padrão já vem por substituir a tecla Tab por quatro espaços mas, quando contrário, normalmente ele é configurável pra trocar (e ninguém precisar apertar realmente a tecla de espaço quatro vezes).

Por exemplo: ~(tente selecionar com o mouse e perceba que a indentação do código de cima é diferente do código de baixo, pois a primeira é composta de um caracter atômico a cada nível de indentação, e seu tamanho irá variar visualmente de editor para editor, inclusive fazendo alguns trechos do código parecerem bizarros ao serem visualizados no GitHub)~

if 'attachments' in message:
    if 'payload' in message['attachments'][0]:
        if 'coordinates' in message['attachments'][0]['payload']:

será diferente de

if 'attachments' in message:
    if 'payload' in message['attachments'][0]:
        if 'coordinates' in message['attachments'][0]['payload']:

que terá o mesmíssimo efeito que usar o operador lógico "e", que somente irá avaliar as operações da direita se a da esquerda retornar um valor conversível a True, sendo essa cascada desnecessária (o else dá a impressão de necessidade? daqui a um ou dois parágrafos há uma solução melhor).

Se o tamanho da linha lhe preocupar, novamente as convenções (que trazem uma dica a nível de programação) irão ajudar.

PS: esse aninhamento potencialmente tá aumentando a complexidade do código; não sei como o Python faz, mas provavelmente percorre todo o array até encontrar o valor. O acesso puro por variáveis é mais eficiente (já que ele encontra somando a posição da memória inicial com a posição do array e, assim, não percorre nada). Outra observação, é que o código parece não dever funcionar na ausência desses valores. Dito isso, talvez seria interessante substituir por um try-catch-finally que capture KeyError ou então use valor um default como None ao armazenar a variável e posteriormente cheque if my_variable is not None. (Se o Python fizer alguma mágica a baixo nível e o algoritmo de busca for não sequencial e o dicionário for pequeno, então desconsidere)

Por falar em KeyError, apenas tratar a superclasse Exception é uma terrível prática. Neste caso, o try-catch poderia estar sendo para direcionar melhor (ou seja, realmente tratar e não apenas engolir imprimindo o stack trace) exceções de IO e o próprio KeyError ao invés de simplesmente Exception. Leia aqui um ou dois truques dos velhos programadores Java.

A maioria dessas preocupações seriam melhor aproveitadas após um refatoramento do código pra modularizá-lo melhor. Quebrar funções em outras (exemplo, um função getter para que, caso entre no catch do keyerror, retorne None, mas se passar pelo try de boas retorne a string que você busca). E se esta modularização for ainda confusa por demais, quebre-a em modules e as importe quando convir. Reza a lenda:

Se um escopo atinge mais de 20 linhas, é porque ele faz "uma coisa" e "outra" e deve ser quebrado em funções menores de faca_coisa() e faca_outra_coisa().

Outras questões menores, é que em Python 2, o print não é método, logo print("Mensagem") vai contra as convenções do 2 (mas é obrigatório no 3). Sendo um statement, se comporta como um break, return, import etc., aí é desnecessário e anti-padrão usar parênteses redundantes em situações simples, talvez apenas usar print ("Mensagem") quando convir um uso mais complexo, pra deixar claro o que é método e o que é statement (em alguns trechos a documentação se confunde quanto a isso, no entanto).

Mazuh commented 7 years ago

Sugestão que resolveria quase tudo: VS Code com plugin PyLint instalado.

felipemfp commented 7 years ago

Vem pro lado Atom da força

Mazuh commented 7 years ago

Juro que nunca nem toquei no Atom. kk

Escolhi o VS Code ao invés dele porque o Code mostra desempenho ligeiramente melhor. E no meu PC (uma porra) cada otimização de performance é válida.

Se eu fosse trocar, trocaria pelo Sublime que tem um desempenho e uso de RAM absurdamente maior que Atom e Code juntos. Mas não me sinto confortável com ele por ser código fechado e eu não conseguir pagar a licença (apesar de usar a versão "grátis" mesmo na faculdade).

Mazuh commented 7 years ago

@yuriscosta, incrementando o comentei no primeiro parágrafo da issue. Há, de fato, vários tipos diferentes de indentação no mesmo arquivo. O que seria pior do que usar apenas tabs.