Semakin banyak pengembang mengerjakan aplikasi dalam tim dan semakin baik tahu kodenya, semakin sering ia terlibat dalam proofreading karya rekan-rekannya. Hari ini saya akan menunjukkan apa yang bisa ditangkap dalam satu minggu dalam kode yang ditulis oleh pengembang yang sangat baik. Di bawah potongan adalah koleksi artefak yang jelas dari kreativitas kita (dan sedikit dari pikiranku).
Pembanding
Ada kode:
@Getter class Dto { private Long id; }
Seseorang mencatat bahwa Anda dapat mengurutkan aliran secara langsung, tetapi saya ingin menarik perhatian Anda ke pembanding. Dokumentasi untuk metode Comparator::compare mengatakan dalam bahasa Inggris dengan warna putih:
Membandingkan dua argumennya untuk pesanan. Mengembalikan bilangan bulat negatif, nol, atau bilangan bulat positif karena argumen pertama kurang dari, sama dengan, atau lebih besar dari yang kedua.
Perilaku inilah yang diterapkan dalam kode kita. Apa yang salah Faktanya adalah bahwa pencipta Jawa sangat jauh-jauh hari menyarankan bahwa banyak yang akan membutuhkan pembanding dan membuatnya untuk kita. Kita dapat menggunakannya dengan menyederhanakan kode kami:
List<Long> readSortedIds(List<Dto> list) { List<Long> ids = list.stream().map(Dto::getId).collect(Collectors.toList()); ids.sort(Comparator.naturalOrder()); return ids; }
Demikian pula kode ini
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; }
dengan sentakan pergelangan tangan berubah menjadi
List<Dto> sortDtosById(List<Dto> list) { list.sort(Comparator.comparing(Dto::getId)); return list; }
Ngomong-ngomong, di "Ideas" versi baru, Anda dapat melakukan seperti ini:
Pelecehan opsional
Mungkin kita masing-masing melihat sesuatu seperti ini:
List<UserEntity> getUsersForGroup(Long groupId) { Optional<Long> optional = Optional.ofNullable(groupId); if (optional.isPresent()) { return userRepository.findUsersByGroup(optional.get()); } return Collections.emptyList(); }
Seringkali Optional digunakan untuk memeriksa ada / tidaknya nilai, meskipun itu tidak dibuat untuk ini. Mengikat penyalahgunaan dan menulis lebih mudah:
List<UserEntity> getUsersForGroup(Long groupId) { if (groupId == null) { return Collections.emptyList(); } return userRepository.findUsersByGroup(groupId); }
Ingat bahwa Optional bukan tentang argumen metode atau bidang, tetapi tentang nilai pengembalian. Itu sebabnya dirancang tanpa dukungan serialisasi.
membatalkan metode yang mengubah keadaan argumen
Bayangkan metode seperti ini:
@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); } }
Tentunya Anda telah melihat dan menulis sesuatu yang serupa berkali-kali. Di sini saya tidak suka bahwa metode mengubah keadaan entitas, tetapi tidak mengembalikannya. Bagaimana cara kerja metode kerangka kerja yang serupa? Misalnya, org.springframework.data.jpa.repository.JpaRepository::save dan javax.persistence.EntityManager::merge mengembalikan nilai. Misalkan setelah memperbarui kontrak kita harus mendapatkannya di luar metode update . Ternyata sesuatu seperti ini:
@Transactional public void anotherMethod(Long contractId, Dto contractDto) { updateService.updateContractById(contractId, contractDto); Contract contract = repositoroty.findOne(contractId); doSmth(contract); }
Ya, kami dapat mengirimkan entitas secara langsung ke metode UpdateService::updateContract dengan mengubah tanda tangannya, tetapi lebih baik melakukannya seperti ini:
@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); } }
Di satu sisi, ini membantu menyederhanakan kode, di sisi lain, ini membantu dengan pengujian. Secara umum, pengujian metode void adalah tugas yang sangat suram, yang akan saya tunjukkan menggunakan contoh yang sama:
@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);
Tapi semuanya bisa dibuat lebih sederhana jika metode mengembalikan nilai:
Pastikan @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()); } }
Dalam sekali tegukan, tidak hanya panggilan ke ContractRepository::save diperiksa, tetapi juga kebenaran dari nilai yang disimpan.
Konstruksi sepeda
Untuk bersenang-senang, buka proyek Anda dan cari ini:
lastIndexOf('.')
Dengan probabilitas tinggi, seluruh ekspresi terlihat seperti ini:
String fileExtension = fileName.subString(fileName.lastIndexOf('.'));
Itulah yang tidak dapat diperingatkan oleh penganalisa statis, ini tentang sepeda yang baru ditemukan. Tuan-tuan, jika Anda memecahkan masalah tertentu yang terkait dengan nama file / ekstensi atau jalan untuk itu, seperti membaca / menulis / menyalin, maka dalam 9 kasus dari 10 tugas telah diselesaikan sebelum Anda. Karena itu, ikat dengan konstruksi sepeda dan ambil solusi yang siap pakai (dan terbukti):
import org.apache.commons.io.FilenameUtils;
Dalam hal ini, Anda menghemat waktu yang akan dihabiskan untuk memeriksa kesesuaian sepeda, dan juga mendapatkan fungsionalitas yang lebih maju (lihat dokumentasi FilenameUtils::getExtension ).
Atau di sini adalah kode yang menyalin isi dari satu file ke file lain:
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); }
Keadaan apa yang bisa mencegah kita? Ribuan dari mereka:
- tujuan mungkin folder, bukan file sama sekali
- sumbernya mungkin folder
- sumber dan tujuan mungkin file yang sama
- tujuan tidak dapat dibuat
- dll dan seterusnya.
Yang menyedihkan adalah bahwa menggunakan self-recording tentang semua itu sudah kita pelajari selama menyalin.
Jika kita melakukannya dengan bijak
import org.apache.commons.io.FileUtils; try { FileUtils.copyFile(src, new File(targetName)); } catch (IOException ex) { log.error("", ex); }
kemudian bagian dari pemeriksaan akan dilakukan sebelum mulai menyalin, dan pengecualian yang mungkin akan lebih informatif (lihat sumber FileUtils::copyFile ).
Mengabaikan @ Nullable / @ NotNull
Misalkan kita memiliki entitas:
@Entity @Getter public class UserEntity { @Id private Long id; @Column private String email; @Column private String petName; }
Dalam kasus kami, kolom email dalam tabel dijelaskan sebagai not null , tidak seperti petName. Artinya, kita dapat menandai bidang dengan anotasi yang sesuai:
import javax.annotation.Nullable; import javax.annotation.NotNull;
Sekilas, ini seperti petunjuk bagi pengembang, dan memang benar. Pada saat yang sama, anotasi ini adalah alat yang jauh lebih kuat daripada label biasa.
Misalnya, lingkungan pengembangan memahaminya, dan jika, setelah menambahkan anotasi, kami mencoba melakukannya seperti ini:
boolean checkIfPetBelongsToUser(UserEnity user, String lostPetName) { return user.getPetName().equals(lostPetName); }
maka "Ide" akan memperingatkan kita tentang bahaya dengan pesan ini:
Metode doa 'sama dengan' dapat menghasilkan 'NullPointerException'
Dalam kode
boolean hasEmail(UserEnity user) { boolean hasEmail = user.getEmail() == null; return hasEmail; }
akan ada peringatan lain:
Syarat 'user.getEmail () == null' selalu 'salah'
Ini membantu sensor bawaan menemukan kemungkinan kesalahan dan membantu kami lebih memahami eksekusi kode. Untuk tujuan yang sama, penjelasan berguna untuk menempatkan metode yang mengembalikan nilai dan argumennya.
Jika argumen saya tampaknya tidak dapat disimpulkan, maka lihatlah kode sumber dari setiap proyek serius, "Musim Semi" yang sama - mereka digantung dengan anotasi seperti pohon Natal. Dan ini bukan kemauan, tetapi kebutuhan yang parah.
Satu-satunya kelemahan, menurut saya, adalah kebutuhan untuk terus memelihara anotasi dalam keadaan modern. Meskipun, jika Anda melihatnya, ini merupakan berkah, karena kembali ke kode berulang-ulang kami memahaminya dengan lebih baik dan lebih baik.
Kecerobohan
Tidak ada kesalahan dalam kode ini, tetapi ada kelebihan:
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());
Tidak jelas mengapa membuat daftar kunci baru yang dengannya pencarian dilakukan, jika itu tidak berubah ketika melewati aliran. Bagus hanya ada 5 elemen, dan jika ada 100500? Dan jika metode getDtos mengembalikan 100500 objek (dalam daftar!), Lalu seperti apa kinerja kode ini? Tidak ada, jadi lebih baik seperti ini:
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());
Juga di sini:
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))); }
Jelas, nilai yang dikembalikan oleh ekspresi inParameterMap.keySet() tidak berubah, sehingga dapat dipindahkan ke variabel:
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))); }
Ngomong-ngomong, bagian tersebut dapat dihitung menggunakan cek 'Alokasi objek dalam satu lingkaran'.
Ketika analisis statis tidak berdaya
Jawa kedelapan sudah lama mati, tapi kita semua suka aliran. Beberapa dari kita sangat mencintai mereka sehingga kita menggunakannya di mana-mana:
public Optional<EmailAdresses> getUserEmails() { Stream<UserEntity> users = getUsers().stream(); if (users.count() == 0) { return Optional.empty(); } return users.findAny(); }
Aliran, seperti yang Anda ketahui, baru sebelum penghentian dipanggil, jadi mengakses kembali variabel users dalam kode kami akan menghasilkan IllegalStateException .
Analis statis belum tahu bagaimana melaporkan kesalahan tersebut, oleh karena itu, tanggung jawab untuk menangkap tepat waktu berada di tangan pengulas.
Tampak bagi saya bahwa menggunakan variabel tipe Stream , serta melewatkannya sebagai argumen dan kembali dari metode, seperti berjalan di ladang ranjau. Mungkin beruntung, mungkin tidak. Karenanya aturan sederhana: setiap tampilan Stream<T> dalam kode harus diperiksa (tetapi dengan cara yang baik segera dipotong).
Tipe sederhana
Banyak yang yakin bahwa boolean , int , dll., Hanya tentang kinerja. Ini sebagian benar, tetapi di samping itu, tipe sederhana not null secara default. Jika bidang bilangan bulat dari entitas mengacu pada kolom yang dinyatakan dalam tabel sebagai not null , maka masuk akal untuk menggunakan int daripada bilangan bulat. Ini adalah semacam kombo - dan konsumsi memori lebih rendah, dan kode disederhanakan karena pemeriksaan yang tidak perlu untuk null .
Itu saja. Ingatlah bahwa semua hal di atas bukanlah kebenaran tertinggi, pikirkan dengan kepala Anda sendiri dan secara bermakna mendekati penerapan saran apa pun.