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

Refactor PersonServiceImpl.getPersonAttributeTypes

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: TBD
    • Resolution: Fixed
    • Affects Version/s: Core 2.2.0
    • Fix Version/s: Core 2.2.0
    • Component/s: None
    • Complexity:
      Medium

      Description

      After the first refactoring (see linked issue and merge commit) of PersonServiceImpl.getPersonAttributeTypes() where

      combineAttributes(String patientAttributeProperty, String userAttributeProperty, PERSON_TYPE personType)
      

      was extracted and duplication removed it becomes clear what we are trying to do and that this can be simplified even further:
      we are

      • getting attributes from two global properties and put them into a string of comma separated attributes
      • we then turn that string of csv into a List<String> which we then use to call other methods in the PersonServiceImpl to get the List<PersonAttributeType> we actually return

      SOLUTION:
      change the

      combineAttributes

      method signature to

      List<String> getAttributeTypesFromGlobalProperties(ATTR_VIEW_TYPE viewType, PERSON_TYPE personType)
      

      why
      1. return the List<String> instead of the String with the attributes separated by commas since thats what we actually need
      2. move the local variable of the AdministrationService

      as

      into the helper method, as well as the logic to determine which global property constant is used to get the attribute types from the DB. Because when you look at

      combineAttributes

      you can see that depending on the PERSON_TYPE we do not actually need to get both attribute properties from the as.getGlobalProperty. So by moving the AdministrationService call with the global properties inside the helper method this can easily be optimized.
      3. Now that

      combineAttributes

      actually does different things, we should rename it to

      getAttributeTypesFromGlobalProperties

      4. remove any comments in the code, also the comment with TODO

      This leads to a cleaner getPersonAttributeTypes which can then look like

      public List<PersonAttributeType> getPersonAttributeTypes(ATTR_VIEW_TYPE viewType, PERSON_TYPE personType) {
      
      if (viewType == null) {
        return Context.getPersonService().getAllPersonAttributeTypes();
      }
      
      List<String> attrNames = getAttributeTypesFromGlobalProperties(viewType, personType);
      
      List<PersonAttributeType> attrObjects = new ArrayList<>();
      		
      if (!attrNames.isEmpty()) {
      ...
      

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

                People

                Assignee:
                fruether Fred Rue
                Reporter:
                teleivo Ivo Ulrich
                Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                  Dates

                  Created:
                  Updated:
                  Resolved: