yrachid
commented
8 years ago Primeira Parte do Code Review
Vou escrevendo o code review aos poucos, para não atrasar vocês por muito tempo. O que eu for encontrando, vou colocando aqui.
0 app/assets/stylesheets/components/Button.scss 73:
- Não é necessário declarar classes em CSS/SCSS que vão ser utilizadas apenas por JavaScript
1 app/assets/javascripts/utilities/only-in-view.js:
- A convenção de nomenclatura deste arquivo parece inconsistente em relação aos demais, pois todos os outros usam snake case (separação por _) nos nomes e este usa traços.
2 app/assets/javascripts/hospital_necessities.js 1:
- Código comentado
3 app/assets/javascripts/hospital_necessities.js 17, 21:
- Código duplicado
4 app/assets/javascripts/hospital_necessities.js 20, 24:
- Sem ponto e vírgula
Sugestões:
Refatorando o hospital_necessities.js
Aproveitem os first class functions do Javascript:
Ao invés de:
var allInputNumbers = document.getElementsByClassName('js-necessityInput');
for(var i = 0; i < allInputNumbers.length; i++){
var inputNumber = allInputNumbers[i];
inputNumber.addEventListener("keyup", function(){
validateFormService.validatePositiveNumber();
validateFormService.validateEmptyInput();
})
inputNumber.addEventListener("click", function(){
validateFormService.validatePositiveNumber();
validateFormService.validateEmptyInput();
})
}Não poderia ser como abaixo?
var allInputNumbers = document.getElementsByClassName('js-necessityInput');
var validateOnAction = function() {
validateFormService.validatePositiveNumber();
validateFormService.validateEmptyInput();
};
//For each maneiro
allInputNumbers.forEach(function(input) {
input.addEventListener("keyup", validateOnAction);
input.addEventListener("click", validateOnAction);
});Ou, ainda, poderia-se criar uma função que faça binding de múltiplos eventos à uma função de uma vez só (isso me parece interessante, pois vai ter bastante reuso desta função em outros scripts):
var bindMultiple = function(component, events, action) {
events.split(' ').forEach(function(event) {
component.addEventListener(event, action);
});
};
var validateOnAction = function() {
validateFormService.validatePositiveNumber();
validateFormService.validateEmptyInput();
};
allInputNumbers.forEach(function(input) {
bindMultiple(input, "keyup click", validateOnAction);
});Ou
var bindMultiple = function(component, events, action) {
events.split(' ').forEach(function(event) {
component.addEventListener(event, action);
});
};
allInputNumbers.forEach(function(input) {
bindMultiple(input, "keyup click", validateFormService.validatePositiveNumber);
bindMultiple(input, "keyup click", validateFormService.validateEmptyInput);
});- Adotar um linter para JavaScript (eslint ou jshint, por exemplo). Vai ajudar o time a manter o código uniforme e com menos erros pequenos (linhas sem ponto e vírgula, inconsistência de aspas, indentação, etc.)
- Tunar o atom com linter. Executar linhas 4 e 14
COMO: Kátia QUERO: Solicitar os doadores de determinados tipo sanguíneo. PARA: Conseguir mais doações para completar o estoque de sangue.
Cenários:
Tarefas:
Notas:
Detalhes técnicos: