صيد بلدي في أسبوع

كلما عمل المطور على تطبيق في فريق وكلما كان يعرف رمزه بشكل أفضل ، كلما كان أكثر انخراطًا في تدقيق عمل رفاقه. اليوم سأعرض ما يمكن ملاحظته في أسبوع واحد في التعليمات البرمجية المكتوبة من قبل المطورين الجيدين للغاية. تحت القطع مجموعة من القطع الأثرية الحية لإبداعنا (وقليلاً من أفكاري).

المقارنات

يوجد رمز:

@Getter class Dto { private Long id; } // another class List<Long> readSortedIds(List<Dto> list) { List<Long> ids = list.stream().map(Dto::getId).collect(Collectors.toList()); ids.sort(new Comparator<Long>() { public int compare(Long o1, Long o2) { if (o1 < o2) return -1; if (o1 > o2) return 1; return 0; } }); return ids; } 

لاحظ شخص ما أنه يمكنك فرز الدفق مباشرة ، لكني أود أن ألفت انتباهك إلى المقارنة. تقول وثائق طريقة Comparator::compare باللغة الإنجليزية بالأبيض:

يقارن بين حجته للنظام. إرجاع عدد صحيح سالب أو صفر أو عدد صحيح موجب لأن الوسيطة الأولى أقل من الثانية أو مساوية لها أو أكبر منها.

هذا هو السلوك الذي يتم تنفيذه في الكود الخاص بنا. ما الخطب؟ والحقيقة هي أن مبدعي جافا جاؤوا ببعد النظر إلى أن الكثيرين سيحتاجون إلى مثل هذا المقارنة وجعلوه لنا. يمكننا استخدامه من خلال تبسيط الكود الخاص بنا:

 List<Long> readSortedIds(List<Dto> list) { List<Long> ids = list.stream().map(Dto::getId).collect(Collectors.toList()); ids.sort(Comparator.naturalOrder()); return ids; } 

وبالمثل ، هذا الرمز

 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; } 

بنقرة من المعصم تتحول إلى

 List<Dto> sortDtosById(List<Dto> list) { list.sort(Comparator.comparing(Dto::getId)); return list; } 

بالمناسبة ، في الإصدار الجديد من "الأفكار" سيكون من الممكن القيام بهذا:

إساءة اختيارية

ربما رأى كل منا شيئًا مثل هذا:

 List<UserEntity> getUsersForGroup(Long groupId) { Optional<Long> optional = Optional.ofNullable(groupId); if (optional.isPresent()) { return userRepository.findUsersByGroup(optional.get()); } return Collections.emptyList(); } 

في كثير من الأحيان ، Optional استخدام Optional للتحقق من وجود / عدم وجود قيمة ، على الرغم من أنه لم يتم إنشاؤه لهذا الغرض. اربط إساءة الاستخدام والكتابة بشكل أسهل:

 List<UserEntity> getUsersForGroup(Long groupId) { if (groupId == null) { return Collections.emptyList(); } return userRepository.findUsersByGroup(groupId); } 

تذكر أن Optional لا تتعلق بوسيطة الأسلوب أو الحقل ، ولكنها تتعلق بالقيمة المرجعة. هذا هو السبب في أنه تم تصميمه بدون دعم التسلسل.

طرق باطلة تغير حالة الجدل

تخيل طريقة مثل هذه:

 @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); } } 

بالتأكيد لقد رأيت وكتبت شيئًا مشابهًا عدة مرات. هنا لا أحب أن الطريقة تغير حالة الكيان ، لكنها لا تعيدها. كيف تتصرف طرق الإطار المماثلة؟ على سبيل المثال ، org.springframework.data.jpa.repository.JpaRepository::save and javax.persistence.EntityManager::merge تُرجع قيمة. لنفترض أنه بعد تحديث العقد ، نحتاج إلى إخراجه من طريقة update . اتضح شيء مثل هذا:

 @Transactional public void anotherMethod(Long contractId, Dto contractDto) { updateService.updateContractById(contractId, contractDto); Contract contract = repositoroty.findOne(contractId); doSmth(contract); } 

نعم ، يمكننا تمرير الكيان مباشرة إلى طريقة UpdateService::updateContract عن طريق تغيير توقيعه ، ولكن من الأفضل القيام بذلك على UpdateService::updateContract :

 @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); } } // @Transactional public void anotherMethod(Long contractId, Dto contractDto) { Contract contract = updateService.updateContractById(contractId, contractDto); doSmth(contract); } 

من ناحية ، هذا يساعد على تبسيط الكود ، من ناحية أخرى ، يساعد في الاختبار. بشكل عام ، يعد اختبار طرق void مهمة كئيبة للغاية ، والتي سأعرضها باستخدام نفس المثال:

 @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); //void // ,       dto? -  : ArgumentCaptor<Contract> captor = ArgumentCaptor.forClass(Contract.class); verify(repository).save(captor.capture()); Contract updated = captor.getValue(); assertEquals(dto.getValue(), updated.getValue()); } } 

ولكن يمكن جعل كل شيء أبسط إذا أعادت الطريقة قيمة:

 @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()); } } 

في ضربة واحدة ، لم ContractRepository::save التحقق فقط من استدعاء ContractRepository::save ، ولكن أيضًا صحة القيمة المحفوظة.

بناء الدراجات

من أجل المتعة ، افتح مشروعك وابحث عن هذا:

 lastIndexOf('.') 

مع احتمال كبير ، يبدو التعبير بأكمله شيئًا مثل هذا:

 String fileExtension = fileName.subString(fileName.lastIndexOf('.')); 

هذا ما لا يمكن لأي محلل ثابت أن يحذر منه ، إنه يتعلق بالدراجة التي تم اختراعها حديثًا. أيها السادة ، إذا كنت تحل مشكلة معينة تتعلق باسم / امتداد الملف أو المسار إليه ، تمامًا مثل القراءة / الكتابة / النسخ ، ففي 9 حالات من أصل 10 تم حل المهمة بالفعل أمامك. لذلك ، اربط مع بناء الدراجات واتخذ الحلول الجاهزة (والمثبتة):

 import org.apache.commons.io.FilenameUtils; //... String fileExtension = FilenameUtils.getExtension(fileName); 

في هذه الحالة ، يمكنك توفير الوقت الذي FilenameUtils::getExtension في التحقق من مدى ملاءمة الدراجة ، وكذلك الحصول على وظائف أكثر تقدمًا (راجع وثائق FilenameUtils::getExtension ).

أو هنا رمز ينسخ محتويات ملف إلى آخر:

 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); } 

ما هي الظروف التي يمكن أن تمنعنا؟ الآلاف منهم:

• قد تكون الوجهة مجلدًا وليس ملفًا على الإطلاق
• قد يكون المصدر مجلد
• قد يكون المصدر والوجهة نفس الملف
• لا يمكن إنشاء الوجهة
• وما إلى ذلك.

الشيء المحزن هو أنه باستخدام التسجيل الذاتي حول كل ما نتعلمه بالفعل أثناء النسخ.
إذا فعلنا ذلك بحكمة

 import org.apache.commons.io.FileUtils; try { FileUtils.copyFile(src, new File(targetName)); } catch (IOException ex) { log.error("", ex); } 

ثم سيتم تنفيذ جزء من عمليات التحقق قبل بدء النسخ ، وسيكون الاستثناء المحتمل أكثر FileUtils::copyFile (راجع مصادر FileUtils::copyFile ).

الإهمال @ Nullable / @ NotNull

لنفترض أن لدينا كيانًا:

 @Entity @Getter public class UserEntity { @Id private Long id; @Column private String email; @Column private String petName; } 

في حالتنا ، يوصف عمود email في الجدول بأنه not null ، على عكس petName. أي أنه يمكننا تمييز الحقول بالتعليقات التوضيحية المقابلة:

 import javax.annotation.Nullable; import javax.annotation.NotNull; //... @Column @NotNull private String email; @Column @Nullable private String petName; 

للوهلة الأولى ، يبدو هذا بمثابة تلميح للمطور ، وهو حقًا. في الوقت نفسه ، تعد هذه التعليقات التوضيحية أداة أكثر قوة من التسمية العادية.

على سبيل المثال ، تفهمها بيئات التطوير ، وإذا حاولنا بعد إضافة التعليقات التوضيحية القيام بذلك على النحو التالي:

 boolean checkIfPetBelongsToUser(UserEnity user, String lostPetName) { return user.getPetName().equals(lostPetName); } 

ثم تحذرنا "الفكرة" من الخطر بهذه الرسالة:

استدعاء الأسلوب "يساوي" قد ينتج "NullPointerException"

في الكود

 boolean hasEmail(UserEnity user) { boolean hasEmail = user.getEmail() == null; return hasEmail; } 

سيكون هناك تحذير آخر:

الشرط "user.getEmail () == null" دائمًا "false"

يساعد ذلك الرقيب المدمج في العثور على الأخطاء المحتملة ويساعدنا على فهم تنفيذ التعليمات البرمجية بشكل أفضل. لنفس الغرض ، فإن التعليقات التوضيحية مفيدة لوضعها على الطرق التي تُرجع القيمة والحجج الخاصة بها.

إذا بدت حججي غير حاسمة ، فقم بإلقاء نظرة على شفرة المصدر لأي مشروع جاد ، وهو نفس "الربيع" - حيث يتم تعليقها باستخدام التعليقات التوضيحية مثل شجرة عيد الميلاد. وهذه ليست نزوة ، بل ضرورة شديدة.

يبدو لي أن العيب الوحيد هو الحاجة إلى الحفاظ على التعليقات التوضيحية باستمرار في حالة حديثة. على الرغم من ذلك ، إذا نظرت إليها ، فهي نعمة ، لأن العودة إلى الشفرة مرارًا وتكرارًا نفهمها بشكل أفضل.

الإهمال

لا توجد أخطاء في هذا الرمز ، ولكن هناك فائض:

 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()); 

ليس من الواضح سبب إنشاء قائمة مفاتيح جديدة يتم من خلالها البحث ، إذا لم يتغير عند المرور عبر الدفق. من الجيد أن هناك 5 عناصر فقط ، ولكن ماذا لو كان هناك 100500 عنصر منهم؟ وإذا getDtos طريقة getDtos 100500 كائن (في القائمة!) ، فما نوع الأداء الذي سيحتوي عليه هذا الرمز؟ لا شيء ، لذلك من الأفضل أن يكون مثل هذا:

 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()); 

أيضا هنا:

 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))); } 

من الواضح أن القيمة التي inParameterMap.keySet() تعبير inParameterMap.keySet() لم تتغير ، لذا يمكن نقلها إلى متغير:

 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))); } 

بالمناسبة ، يمكن حساب هذه الأقسام باستخدام الاختيار "تخصيص كائن في حلقة".

عندما يكون التحليل الثابت عاجزا

لقد ماتت جافا الثامنة منذ فترة طويلة ، لكننا جميعًا نحب الجداول. البعض منا يحبهم كثيرًا حتى نستخدمهم في كل مكان:

 public Optional<EmailAdresses> getUserEmails() { Stream<UserEntity> users = getUsers().stream(); if (users.count() == 0) { return Optional.empty(); } return users.findAny(); } 

الدفق ، كما تعلمون ، جديد قبل استدعاء الإنهاء ، لذا فإن إعادة الوصول إلى متغير users في الكود الخاص بنا سيؤدي إلى IllegalStateException .

لا يعرف المحللون الثابتون حتى الآن كيفية الإبلاغ عن مثل هذه الأخطاء ، وبالتالي ، تقع مسؤولية التقاطها في الوقت المناسب على عاتق المراجع.

يبدو لي أن استخدام متغيرات من نوع Stream ، وكذلك تمريرها كحجج والعودة من الأساليب ، يشبه المشي في حقل ألغام. ربما محظوظ ، ربما لا. ومن هنا قاعدة بسيطة: يجب فحص أي مظهر Stream<T> في الشفرة (ولكن يتم قطعه بطريقة جيدة على الفور).

أنواع بسيطة

كثيرون على يقين من أن boolean ، إلخ ، تتعلق فقط بالأداء. هذا صحيح جزئيًا ، ولكن بالإضافة إلى ذلك ، النوع البسيط not null بشكل افتراضي. إذا كان الحقل الصحيح للكيان يشير إلى عمود تم تعريفه في الجدول على أنه not null ، فمن المنطقي استخدام int بدلاً من Integer . هذا نوع من التحرير والسرد - واستهلاك الذاكرة أقل ، ويتم تبسيط الرمز بسبب عمليات الفحص غير الضرورية null .

هذا كل شيء. تذكر أن كل ما سبق ليس الحقيقة المطلقة ، فكر برأسك واعمل بشكل هادف على تطبيق أي نصيحة.