Desafio artur veloso#19
Conversation
| verbose_name = 'Questão' | ||
| verbose_name_plural = 'Questões' | ||
|
|
||
| class Alternatives(models.Model): |
There was a problem hiding this comment.
Boa 👍
Usou a nomenclatura certa para as coisas não ficarem confusas.
Answer sempre foi uma escolha pior, porque aqui o que importa são as alternativas da questão, as respostas são uma coisa ligada ao usuário e assim devem estar relacionadas à tabela de ligação (neste caso QuestionLogs).
Apenas para lembrar, normalmente se usa nomes no singular para as classes dos modelos 😉
| on_delete=models.CASCADE, | ||
| verbose_name="Questão" | ||
| ) | ||
| alternative_text = models.TextField(verbose_name="Questão") |
There was a problem hiding this comment.
Como estes são atributos de Alternative, não seria necessário prefixá-los com alternative_, chega a ser um pouco redundante:
correct_alternative = Alternatives.objects.filter(question=question, is_correct=True).first()
correct_alternative.alternative_text # soa estranho, não é?
| on_delete=models.CASCADE, | ||
| verbose_name="Questão" | ||
| ) | ||
| chosen_alternative = models.CharField(max_length=4, verbose_name="Alternativa", null=True, blank=True) |
There was a problem hiding this comment.
Olha minha sugestão:
Se ao invés de guardar o valor da alternativa, tivesse usado uma ForeignKey para Alternative, você teria com facilidade o estado de se ela está correta ou não e poderia ter usado uma property para recuperar o is_correct, que no final das contas é um estado transitório e não deveria ser armazenado em cada QuestionLog, não é mesmo?
| verbose_name="Está correta?", | ||
| default=False | ||
| ) | ||
| answer_date = models.DateField(null=True, blank=True) |
There was a problem hiding this comment.
Campos DateTimeField são mais flexíveis, e você poderia ter aproveitado para usar o parâmetro auto_now_add para setar a data automaticamente.
| import datetime | ||
|
|
||
| def home(request): | ||
| questions = Question.objects.all().first() |
There was a problem hiding this comment.
Nomear variáveis é sempre algo sutilmente difícil.
Aqui será retornada no máximo 1 questão (por causa do .first()), então para não causar confusão (nem para os outros nem para si mesmo no futuro) seria melhor chamar a variável de question no singular.
(É impressionante como os nomes podem nos levar a erros de raciocínio, não é mesmo? 🙂 )
| } | ||
|
|
||
| def question(request, id): | ||
| questions = Question.objects.get(id=id) |
There was a problem hiding this comment.
Aqui seria melhor usar o método .get_or_404() do model manager, para garantir que a exceção correta seja usada.
| try: | ||
| answer_id = request.POST.get('answer') | ||
|
|
||
| alternative = Alternatives.objects.get(id=answer_id) |
There was a problem hiding this comment.
Tudo que vem das entradas é suspeito (e nunca se pode confiar totalmente no tratamento feito no front, mas sim devemos complementar esse trabalho).
Usar .get() muito diretamente é arriscado.
Melhor um .get_or_404() também aqui.
| } | ||
|
|
||
| if alternative.question.id < len(questions): | ||
| context['next'] = alternative.question.id + 1 |
There was a problem hiding this comment.
Nada garante que exista um registro com um id imediatamente seguinte ao atual (o registro pode ter sido apagado, por exemplo).
Você poderia pegar todos os registros com id maior que o atual, ordenados por id e pegar o primeiro (esse sim é o próximo id).
| chosen_alternative = alternative.alternative_order, | ||
| is_correct = alternative.is_correct, | ||
| answer_date = datetime.date.today() | ||
| ) |
There was a problem hiding this comment.
Note como esses dois casos (do if e do else) são semelhantes.
Você está chamando QuestionLogs.objects.create() em ambos.
Até mesmo a maioria dos parâmetros são os mesmos.
Uma ideia legal, seria a de apenas criar um dicionário de parâmetros, ajustá-lo conforme o caso e desempacotá-los na chamada (única) de QuestionLogs.objects.create().
| ) | ||
|
|
||
| return render(request, 'question/answer.html', context=context) | ||
| except: |
There was a problem hiding this comment.
Ah! Mau menino 🙂
Não se deve usar exceções genéricas sem um tratamento diferenciado.
Isso acaba levando a tratar de uma forma indiscriminada uma exceção que não se tinha previsto.
2 participants
No description provided.