Purpose
A good name variables and methods for understanding the logic of the source code is very important, for yourself and others to maintain the code is a great effect.
This article analysis my own source code's issues, the purpose is to improve the level of code naming.
Overview
I sort out the code that names problems in different types, and I will analyze them one by one.
Use variable names to represent status
- Modification description
Original notificationStatus is replace by isNotificationOn
- Analysis
When you read notificationStatus in code, you are not sure what status happen should go to removeCouponNotificationByCode.But when you read isNotificationOn, you are very sure notification is on will removeCouponNotificationByCode.
Original code:
@RequireHardLogIn
@RequestMapping(value = "/coupon/notification/{code}", method = RequestMethod.GET)
@ResponseBody
public String setNotification(@RequestParam("notificationStatus") final boolean notificationStatus,
@PathVariable("code") final String code, final Model model, final RedirectAttributes redirectModel)
throws CMSItemNotFoundException
{
if (notificationStatus)
{
customerCouponFacade.removeCouponNotificationByCode(code);
}
else
{
customerCouponFacade.saveCouponNotification(code);
}
return "success";
}
Modified code:
@RequireHardLogIn
@RequestMapping(value = "/coupon/notification/{code}", method = RequestMethod.GET)
@ResponseBody
public String setNotification(@RequestParam("isNotificationOn") final boolean isNotificationOn,
@PathVariable("code") final String code, final Model model, final RedirectAttributes redirectModel)
throws CMSItemNotFoundException
{
if (isNotificationOn)
{
customerCouponFacade.removeCouponNotificationByCode(code);
}
else
{
customerCouponFacade.saveCouponNotification(code);
}
return "success";
}
Shorthand mislead
- Modification description
Original findExPromotionSourceRuleByProduct is replace by findExcluPromotionSourceRuleByProduct
- Analysis
When you read findExPromotionSourceRuleByProduct in code, without definition message,you may be think about previous or extend.When you read exclu will not be mislead.
Original code:
@Override
public List<PromotionSourceRuleModel> findExPromotionSourceRuleByProduct(final String productCode)
{
validateParameterNotNull(productCode, "Product code must not be null");
final FlexibleSearchQuery query = new FlexibleSearchQuery(FIND_EXCLUDED_PROMOTION_RULE_FOR_PRODUCT);
query.addQueryParameter("productCode", productCode);
return getFlexibleSearchService().<PromotionSourceRuleModel> search(query).getResult();
}
Modified code:
@Override
public List<PromotionSourceRuleModel> findExcluPromotionSourceRuleByProduct(final String productCode)
{
validateParameterNotNull(productCode, "Product code must not be null");
final FlexibleSearchQuery query = new FlexibleSearchQuery(FIND_EXCLUDED_PROMOTION_RULE_FOR_PRODUCT);
query.addQueryParameter("productCode", productCode);
return getFlexibleSearchService().<PromotionSourceRuleModel> search(query).getResult();
}
Wrong grammar
- Modification description
Original getBindedPromotionSourceRuleByProduct is replace by getBindingPromotionSourceRuleByProduct
- Analysis
The wrong syntax should be avoided in naming.
Original code:
protected List<PromotionSourceRuleModel> getBindedPromotionSourceRuleByProduct(final ProductModel product)
{
final List<PromotionSourceRuleModel> promotionSourceRuleList = new ArrayList<>();
promotionSourceRuleList.addAll(getCustomerCouponService().getPromotionSourceRulesForProduct(product.getCode()));
getCategoryDao().findCategoriesByCatalogVersionAndProduct(product.getCatalogVersion(), product).stream()
.forEach(category -> {
final Collection<CategoryModel> superCategorys = categoryService.getAllSupercategoriesForCategory(category)
.isEmpty() ? new ArrayList<>() : categoryService.getAllSupercategoriesForCategory(category);
superCategorys.add(category);
Modified code:
protected List<PromotionSourceRuleModel> getBindingPromotionSourceRuleByProduct(final ProductModel product)
{
final List<PromotionSourceRuleModel> promotionSourceRuleList = new ArrayList<>();
promotionSourceRuleList.addAll(getCustomerCouponService().getPromotionSourceRulesForProduct(product.getCode()));
getCategoryDao().findCategoriesByCatalogVersionAndProduct(product.getCatalogVersion(), product).stream()
.forEach(category -> {
final Collection<CategoryModel> superCategorys = categoryService.getAllSupercategoriesForCategory(category)
.isEmpty() ? new ArrayList<>() : categoryService.getAllSupercategoriesForCategory(category);
superCategorys.add(category);
Order of words
- Modification description
Original isCustomerCouponAssigned is replace by isUnassignedCoupon
- Analysis
The correct order allows us to better understand the content, and other variables and consistent with the name appears to be more standardized.
Original code:
couponNotifications.stream().forEach(couponNotification -> {
if (couponNotification.getStatus().equals(CouponNotificationStatus.EXPIRESENT))
final boolean isCustomerCouponAssigned = getCustomerCouponDao().checkCustomerCouponAsignedForCustomer(
couponNotification.getCustomerCoupon().getCouponId(), couponNotification.getCustomer());
Modified code:
couponNotifications.stream().forEach(couponNotification -> {
if (couponNotification.getStatus().equals(CouponNotificationStatus.EXPIRESENT))
final boolean isUnassignedCoupon = getCustomerCouponDao().checkCustomerCouponAsignedForCustomer(
couponNotification.getCustomerCoupon().getCouponId(), couponNotification.getCustomer());
