#390: Refatorando importadores CONV

[hpe95]

Resolve #390

[coveralls]

Pull Request Test Coverage Report for Build 983

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 145 unchanged lines in 5 files lost coverage.
• Overall coverage decreased (-3.3%) to 78.705%

---

Files with Coverage Reduction	New Missed Lines	%
modelagem/models.py	2	96.03%
importadores/conv.py	4	95.76%
analises/tests/tests_analise.py	38	75.32%
modelagem/tests/tests_utils.py	39	80.6%
analises/analise.py	62	69.94%

Totals
Change from base Build 978:	-3.3%
Covered Lines:	3696
Relevant Lines:	4696

---

💛 - Coveralls

[diraol]

Pessoal,

primeiro que não faz sentido usar um dicionário (_votacao_params) e tratar as "keys" do dicionário como "índices".

Segundo que ao invés de usar for i in range(len(self._votacao_params)) e depois ficar acessando os "índices", podemos ser mais diretos com python e "loopar" diretamente nos itens.

Exexemplo didático caso _votacao_params seja um dicionário:

_votacao_params = {
    1: ['Reforma agrária',
         [models.SIM, models.ABSTENCAO, models.NAO],
         [models.SIM, models.SIM, models.SIM],
         [models.NAO, models.NAO, models.NAO]],
    2: ['Aumento da pensão dos nobres',
         [models.NAO, models.NAO, models.NAO],
         [models.NAO, models.NAO, models.NAO],
         [models.SIM, models.SIM, models.SIM]]
}

for key, value in _votacao_params.items():
    list_params = value
    numero_prop = int(key)
    descricao_prop = list_params[0]
    prop = self._gera_proposicao(numero_prop, descricao_prop)
    votacao = self._gera_votacao(
        numero_prop, descricao_prop,
        DATA_NO_PRIMEIRO_SEMESTRE, prop)
    self._gera_votos(votacao, GIRONDINOS, list_params[1])
    self._gera_votos(votacao, JACOBINOS, list_params[2])
    self._gera_votos(votacao, MONARQUISTAS, list_params[3])

Outra opção é usar uma lista no lugar do dicionário:

_votacao_params = [
    ['Reforma agrária',
     [models.SIM, models.ABSTENCAO, models.NAO],
     [models.SIM, models.SIM, models.SIM],
     [models.NAO, models.NAO, models.NAO]
    ],
    ['Aumento da pensão dos nobres',
     [models.NAO, models.NAO, models.NAO],
     [models.NAO, models.NAO, models.NAO],
     [models.SIM, models.SIM, models.SIM]
    ]
]

for index, item in enumerate(_votacao_params):
    list_params = item
    numero_prop = int(index+1)
    descricao_prop = list_params[0]
    prop = self._gera_proposicao(numero_prop, descricao_prop)
    votacao = self._gera_votacao(
        numero_prop, descricao_prop,
        DATA_NO_PRIMEIRO_SEMESTRE, prop)
    self._gera_votos(votacao, GIRONDINOS, list_params[1])
    self._gera_votos(votacao, JACOBINOS, list_params[2])
    self._gera_votos(votacao, MONARQUISTAS, list_params[3])

Mas, objetivando ficar com apenas 1 método "gera_votacoes", eu faria algo mais ou menos na seguinte linha:

_votacao_params = [
    {"descricao": 'Reforma agrária',
     "partidos": {
         "GIRONDINOS": [models.SIM, models.ABSTENCAO, models.NAO],
         "JACOBINOS": [models.SIM, models.SIM, models.SIM],
         "MONARQUISTAS": [models.NAO, models.NAO, models.NAO])
]

for index, votacao in enumerate(_votacao_params):
    numero = int(index+1)
    descricao = votacao.get("descricao")
    proposicao = self._gera_proposicao(numero, descricao)
    partidos = votacao.get("partidos")
    votacao = self._gera_votacao(
        numero, descricao, DATA_NO_PRIMEIRO_SEMESTRE, proposicao)
    for partido, votos in partidos.items():
        self._gera_votos(votacao, partido, votos)

Dessa forma vocês conseguem inclusive incluir os dados da "votação8" e remover também o método "gera_proposicao10".

[diraol]

Aliás, pensando melhor aqui..... esses dados do "_votacoes_param" poderiam ficar num arquivo externo (csv ou json) que é lido nesse código que vocês fizeram, ao invés de ficar "hard-coded" dentro do módulo.
(vejam, o codeclimate reclamou das duplicações, e faz sentido, "dados" não devem ficar no source dessa forma. ;)

[diraol]

Essa sintaxe está incorreta em python.

if (indexacao and ementa) is not None: é diferente de if indexacao is not None and ementa is not None:.

Aliás, não sei se os dois precisam se tratados de forma conjunta....
Porque não um if/else para cada, individualmente?

[hpe95]

A ideia era que só estou trabalhando com dois casos:

1. Quando se passa valor pra indexacao e ementa, ou seja, de forma conjunta
2. Quando nao se passa, ai só ementa é tratada e de forma diferente.

Talvez trabalhar de forma individual, seja mais expansivo, mas não sei se é necessário. E também o código fica um pouco mais enxuto. O que você acha?

[diraol]

@peddrro entendi o seu ponto.
A questão é: O método possui os dois argumentos, e de forma "desvinculada".
Se alguém passar "ementa" e não passar "indexação", vai esperar que a "ementa" seja usada - e vice-versa.
Isso talvez gere dúvidas em quem for utilizar o método.
@leonardofl o que você acha?

[hpe95]

@diraol Verdade, que tal eu passar um parametro só entao, que represente os dois para não haver essa "desvinculação" e consequentemente não gerar dúvidas? Acha que seria uma boa?

[leonardofl]

Pode ser uma boa opção. Aí vc cria esse objeto com um construtor que recebe os dois parâmetros (sem valor default), pra dizer que se criar o objeto, tem que usar os dois parâmetros.

Pra não criar uma classe nova, outra opção é criar dois métodos... um que não recebe (indexacao, ementa) e outro que recebe (indexacao, ementa), sem valores defaults. É um tanto quanto "javístico", mas acho q nesse caso tb ajudaria o entendimento de quem for usar o método.

[diraol]

A variável data, gerada na linha anterior pela chamada do método _obtem_dados_json() pode ser None.
Se for, a expressão data["votacoes_params"] vai levantar uma excessão necessariamente. Isso precisa ser tratado corretamente, e isso pode se dar de diversas formas.

Verificando se data is None, retornando um dicionário com valor vazio para a chave "votacoes_params", retornando um dicionário vazio no método e usando o get ao invésde ["votacoes_params"] e definindo um valor default no get... enfim...

Independente disso, eu acho que estão faltando testes para os novos métodos que foram escritos/implementados.

[hpe95]

Ok! Irei fazer as devidas alterações. Obrigado pelas dicas =)