Quanto mais um desenvolvedor trabalha em um aplicativo em uma equipe e melhor conhece seu código, mais frequentemente ele se dedica a revisar o trabalho de seus companheiros. Hoje vou mostrar o que pode ser capturado em uma semana no código escrito por desenvolvedores muito bons. Sob o corte, há uma coleção de artefatos vívidos de nossa criatividade (e um pouco de meus pensamentos).
Comparadores
Existe um código:
@Getter class Dto { private Long id; }
Alguém observa que você pode classificar o fluxo diretamente, mas quero chamar sua atenção para o comparador. A documentação do método Comparator::compare diz em inglês em branco:
Compara seus dois argumentos para ordem. Retorna um número inteiro negativo, zero ou um número inteiro positivo, pois o primeiro argumento é menor que, igual a ou maior que o segundo.
É esse comportamento que é implementado em nosso código. O que está errado? O fato é que os criadores de Java sugeriram muito claramente que muitos precisariam de um comparador e o fizeram para nós. Podemos usá-lo simplificando nosso código:
List<Long> readSortedIds(List<Dto> list) { List<Long> ids = list.stream().map(Dto::getId).collect(Collectors.toList()); ids.sort(Comparator.naturalOrder()); return ids; }
Da mesma forma, esse código
List<Dto> sortDtosById(List<Dto> list) { list.sort(new Comparator<Dto>() { public int compare(Dto o1, Dto o2) { if (o1.getId() < o2.getId()) return -1; if (o1.getId() > o2.getId()) return 1; return 0; } }); return list; }
com um movimento do pulso se transforma em
List<Dto> sortDtosById(List<Dto> list) { list.sort(Comparator.comparing(Dto::getId)); return list; }
A propósito, na nova versão de “Ideas” será possível fazer o seguinte:
Abuso opcional
Provavelmente, cada um de nós viu algo parecido com isto:
List<UserEntity> getUsersForGroup(Long groupId) { Optional<Long> optional = Optional.ofNullable(groupId); if (optional.isPresent()) { return userRepository.findUsersByGroup(optional.get()); } return Collections.emptyList(); }
Freqüentemente, Optional usado para verificar a presença / ausência de um valor, embora não tenha sido criado para isso. Amarre o abuso e escreva mais facilmente:
List<UserEntity> getUsersForGroup(Long groupId) { if (groupId == null) { return Collections.emptyList(); } return userRepository.findUsersByGroup(groupId); }
Lembre-se de que Optional não é sobre o argumento ou campo do método, mas sobre o valor de retorno. É por isso que foi projetado sem suporte à serialização.
métodos nulos que alteram o estado de um argumento
Imagine um método como este:
@Component @RequiredArgsConstructor public class ContractUpdater { private final ContractRepository repository; @Transactional public void updateContractById(Long contractId, Dto contractDto) { Contract contract = repository.findOne(contractId); contract.setValue(contractDto.getValue()); repository.save(contract); } }
Certamente você já viu e escreveu algo semelhante muitas vezes. Aqui não gosto que o método altere o estado da entidade, mas não o retorne. Como se comportam métodos-quadro semelhantes? Por exemplo, org.springframework.data.jpa.repository.JpaRepository::save e javax.persistence.EntityManager::merge retornam um valor. Suponha que, após atualizar um contrato, precisamos tirá-lo do método de update . Acontece algo como isto:
@Transactional public void anotherMethod(Long contractId, Dto contractDto) { updateService.updateContractById(contractId, contractDto); Contract contract = repositoroty.findOne(contractId); doSmth(contract); }
Sim, podemos passar a entidade diretamente para o método UpdateService::updateContract alterando sua assinatura, mas é melhor fazer isso da seguinte maneira:
@Component @RequiredArgsConstructor public class ContractUpdater { private final ContractRepository repository; @Transactional public Contract updateContractById(Long contractId, Dto contractDto) { Contract contract = repository.findOne(contractId); contract.setValue(contractDto.getValue()); return repository.save(contract); } }
Por um lado, isso ajuda a simplificar o código, por outro lado, ajuda nos testes. Em geral, testar métodos void é uma tarefa extremamente sombria, que mostrarei usando o mesmo exemplo:
@RunWith(MockitoJUnitRunner.class) public class ContractUpdaterTest { @Mock private ContractRepository repository; @InjectMocks private ContractUpdater updater; @Test public void updateContractById() { Dto dto = new Dto(); dto.setValue("- "); Long contractId = 1L; when(repository.findOne(contractId)).thenReturn(new Contract()); updater.updateContractById(contractId, contractDto);
Mas tudo pode ser simplificado se o método retornar um valor:
Certifique-se de @RunWith(MockitoJUnitRunner.class) public class ContractUpdaterTest { @Mock private ContractRepository repository; @InjectMocks private ContractUpdater updater; @Test public void updateContractById() { Dto dto = new Dto(); dto.setValue("- "); Long contractId = 1L; when(repository.findOne(contractId)).thenReturn(new Contract()); Contract updated = updater.updateContractById(contractId, contractDto); assertEquals(dto.getValue(), updated.getValue()); } }
De uma só vez, não apenas a chamada para ContractRepository::save verificada, mas também a correção do valor salvo.
Construção de bicicletas
Por diversão, abra seu projeto e procure pelo seguinte:
lastIndexOf('.')
Com uma alta probabilidade, toda a expressão se parece com isso:
String fileExtension = fileName.subString(fileName.lastIndexOf('.'));
É sobre isso que nenhum analisador estático pode alertar, é sobre a bicicleta recém-inventada. Senhores, se você está resolvendo um determinado problema relacionado ao nome / extensão do arquivo ou ao caminho para ele, assim como a leitura / gravação / cópia, em 9 dos 10 casos a tarefa já foi resolvida antes de você. Portanto, amarre a construção da bicicleta e adote soluções prontas (e comprovadas):
import org.apache.commons.io.FilenameUtils;
Nesse caso, você economiza tempo gasto na verificação da adequação da bicicleta e também obtém funcionalidades mais avançadas (consulte a documentação FilenameUtils::getExtension ).
Ou aqui está um código que copia o conteúdo de um arquivo para outro:
try { FileChannel sc = new FileInputStream(src).getChannel(); FileChannel dc = new FileOutputStream(new File(targetName)).getChannel(); sc.transferTo(0, sc.size(), dc); dc.close(); sc.close(); } catch (IOException ex) { log.error("", ex); }
Que circunstâncias podem nos impedir? Milhares deles:
- o destino pode ser uma pasta, não um arquivo
- a fonte pode ser uma pasta
- origem e destino podem ser o mesmo arquivo
- destino não pode ser criado
- etc. e assim por diante.
O triste é que usando a auto-gravação sobre tudo o que aprendemos já durante a cópia.
Se fizermos isso sabiamente
import org.apache.commons.io.FileUtils; try { FileUtils.copyFile(src, new File(targetName)); } catch (IOException ex) { log.error("", ex); }
parte das verificações será realizada antes do início da cópia e uma possível exceção será mais informativa (consulte as fontes de FileUtils::copyFile ).
Negligenciando @ Nullable / @ NotNull
Suponha que tenhamos uma entidade:
@Entity @Getter public class UserEntity { @Id private Long id; @Column private String email; @Column private String petName; }
No nosso caso, a coluna de email na tabela é descrita como not null , diferente de petName. Ou seja, podemos marcar os campos com as anotações correspondentes:
import javax.annotation.Nullable; import javax.annotation.NotNull;
À primeira vista, isso parece uma dica para o desenvolvedor, e realmente é. Ao mesmo tempo, essas anotações são uma ferramenta muito mais poderosa do que um rótulo comum.
Por exemplo, os ambientes de desenvolvimento os entendem e, se depois de adicionar anotações, tentamos fazer o seguinte:
boolean checkIfPetBelongsToUser(UserEnity user, String lostPetName) { return user.getPetName().equals(lostPetName); }
então a "Idéia" nos avisará do perigo com esta mensagem:
A chamada de método 'equals' pode produzir 'NullPointerException'
No código
boolean hasEmail(UserEnity user) { boolean hasEmail = user.getEmail() == null; return hasEmail; }
haverá outro aviso:
A condição 'user.getEmail () == null' é sempre 'false'
Isso ajuda o censor interno a encontrar possíveis erros e nos ajuda a entender melhor a execução do código. Para o mesmo objetivo, as anotações são úteis para colocar em métodos que retornam um valor e seus argumentos.
Se meus argumentos parecem inconclusivos, observe o código-fonte de qualquer projeto sério, o mesmo "Spring" - eles estão marcados com anotações como uma árvore de Natal. E isso não é um capricho, mas uma necessidade grave.
A única desvantagem, me parece, é a necessidade de manter constantemente anotações em um estado moderno. Embora, se você olhar para ele, é uma benção, porque, voltando ao código repetidamente, entendemos cada vez melhor.
Descuido
Não há erros neste código, mas há um excesso:
Collection<Dto> dtos = getDtos(); Stream.of(1,2,3,4,5) .filter(id -> { List<Integer> ids = dtos.stream().map(Dto::getId).collect(toList()); return ids.contains(id); }) .collect(toList());
Não está claro por que criar uma nova lista de chaves pelas quais a pesquisa é realizada, se ela não muda ao passar pelo fluxo. É bom que existam apenas 5 elementos e, se houver 100500 deles? E se o método getDtos retornar 100500 objetos (na lista!), Que tipo de desempenho esse código terá? Nenhum, então é melhor assim:
Collection<Dto> dtos = getDtos(); Set<Integer> ids = dtos.stream().map(Dto::getId).collect(toSet()); Stream.of(1,2,3,4,5) .filter(ids::contains) .collect(toList());
Também aqui:
static <T, Q extends Query> void setParams( Map<String, Collection<T>> paramMap, Set<String> notReplacedParams, Q query) { notReplacedParams.stream() .filter(param -> paramMap.keySet().contains(param)) .forEach(param -> query.setParameter(param, paramMap.get(param))); }
Obviamente, o valor retornado pela expressão inParameterMap.keySet() permanece inalterado, portanto pode ser movido para uma variável:
static <T, Q extends Query> void setParams( Map<String, Collection<T>> paramMap, Set<String> notReplacedParams, Q query) { Set<String> params = paramMap.keySet(); notReplacedParams.stream() .filter(params::contains) .forEach(param -> query.setParameter(param, paramMap.get(param))); }
A propósito, essas seções podem ser calculadas usando a verificação 'Alocação de objetos em um loop'.
Quando a análise estática é impotente
Oitava Java há muito tempo desapareceu, mas todos nós amamos fluxos. Alguns de nós os amam tanto que os usamos em qualquer lugar:
public Optional<EmailAdresses> getUserEmails() { Stream<UserEntity> users = getUsers().stream(); if (users.count() == 0) { return Optional.empty(); } return users.findAny(); }
O fluxo, como você sabe, é novo antes da rescisão ser chamada, portanto, acessar novamente a variável de users em nosso código resultará em uma IllegalStateException .
Os analisadores estáticos ainda não sabem como reportar esses erros; portanto, a responsabilidade pela captura oportuna recai sobre o revisor.
Parece-me que usar variáveis do tipo Stream , além de transmiti-las como argumentos e retornar de métodos, é como caminhar em um campo minado. Talvez com sorte, talvez não. Portanto, uma regra simples: qualquer aparência do Stream<T> no código deve ser verificada (mas de uma maneira boa imediatamente cortada).
Tipos simples
Muitos têm certeza de que boolean , int etc. são apenas sobre desempenho. Isso é parcialmente verdade, mas, além disso, o tipo simples not null é not null por padrão. Se um campo inteiro de uma entidade se referir a uma coluna declarada na tabela como not null , faz sentido usar int vez de Integer . Esse é um tipo de combinação - e o consumo de memória é menor e o código é simplificado devido a verificações desnecessárias de null .
Isso é tudo. Lembre-se de que tudo o que foi exposto acima não é a verdade suprema, pense com sua própria cabeça e aborde significativamente a aplicação de qualquer conselho.