Uploaded image for project: 'OpenMRS Core'
  1. OpenMRS Core
  2. TRUNK-5057

Extract private methods in OrderServiceImpl.saveOrder

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Could
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: Core 2.2.0
    • Component/s: None
    • Labels:
      None
    • Complexity:
      Low

      Description

      To reduce the complexity, increase readability and maintainability in

      OrderServiceImpl.saveOrder(Order)

      I suggest to extract related parts into smaller private methods, with longer meaningful names.

      For example:

      https://github.com/openmrs/openmrs-core/blob/master/api/src/main/java/org/openmrs/api/impl/OrderServiceImpl.java#L133-L135 into

      private void failOnExistingOrder(Order order)
      

      https://github.com/openmrs/openmrs-core/blob/master/api/src/main/java/org/openmrs/api/impl/OrderServiceImpl.java#L136-L138 into

      private void ensureDateActivatedIsSet(Order order)
      

      https://github.com/openmrs/openmrs-core/blob/master/api/src/main/java/org/openmrs/api/impl/OrderServiceImpl.java#L139-L154 into

      private void ensureConceptIsSet(Order order)
      

      https://github.com/openmrs/openmrs-core/blob/master/api/src/main/java/org/openmrs/api/impl/OrderServiceImpl.java#L156-L180 into

      private void ensureOrderTypeIsSet(Order order, OrderContext orderContext)
      

      https://github.com/openmrs/openmrs-core/blob/master/api/src/main/java/org/openmrs/api/impl/OrderServiceImpl.java#L181-L190 into

      private void ensureCareSettingIsSet(Order order, OrderContext orderContext)
      

      https://github.com/openmrs/openmrs-core/blob/master/api/src/main/java/org/openmrs/api/impl/OrderServiceImpl.java#L192-L195 into

      private void failOnOrderTypeMismatch(Order order)
      

      which turns the saveOrder method into

      	private Order saveOrder(Order order, OrderContext orderContext, boolean isRetrospective) {
      		failOnExistingOrder(order);
      		ensureDateActivatedIsSet(order);
      		ensureConceptIsSet(order);
      		ensureOrderTypeIsSet(order, orderContext);
      		ensureCareSettingIsSet(order, orderContext);
      		failOnOrderTypeMismatch(order);
      
      		Order previousOrder = order.getPreviousOrder();
      		if (REVISE == order.getAction()) {
      			if (previousOrder == null) {
      				throw new PreviousOrderRequiredException();
      			}
      			stopOrder(previousOrder, aMomentBefore(order.getDateActivated()), isRetrospective);
      		} else if (DISCONTINUE == order.getAction()) {
      			discontinueExistingOrdersIfNecessary(order, isRetrospective);
      		}
      ...
      

      Keep the private methods in order of their execution and right after the method saveOrder.

      And only extract the private methods without refactoring them as well in order to ease code review and reduce risk of breaking the code. If you have an idea on how to improve the private methods later on submit another PR.

        Attachments

          Activity

            People

            • Assignee:
              nuwantha Nuwantha Dewappriya
              Reporter:
              teleivo Ivo Ulrich
              Designated Committer:
              Burke Mamlin
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Time Tracking

                Estimated:
                Original Estimate - 5 hours
                5h
                Remaining:
                Time Spent - 3 hours Remaining Estimate - 2 hours
                2h
                Logged:
                Time Spent - 3 hours Remaining Estimate - 2 hours
                3h