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

Extract private methods in OrderServiceImpl.saveOrder

    XMLWordPrintable

    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