Switch statement results in duplicated code-CodePudding

I'm a junior dev, and looking to better organize my code.

Right now, I'm dealing with form numbers. I retrieve a hash map of form numbers, and based on the form number, I need to call a different method. Each method takes in the same parameters, but does something different.

For example:


var formDetails = new inferForms.buildFormsMap

for(form in formDetails){

switch(form.formNumber){
          case "A1345":
            getExclusionDetails(account, state, form, businessDealing)
            break
          case "B254":
            getExclusionDetails(account, state, form, businessDealing)
            break
          case "B297":
            getPartnershipDetails(account, state, form, businessDealing)
            break
          case "C397":
            getBrokerageDetails(account, state, form, businessDealing)
            break
          case "D972":
            getBrokerageDetails(account, state, form, businessDealing)
            break
          case "E192":
            getBrokerageDetails(account, state, form, businessDealing)
            break
          case "E299":
            getBrokerageDetails(account, state, form, businessDealing)
            break
          case "F254":
            getLocationDetails(account, state, form, businessDealing)
            break
          case "F795":
            getLocationDetails(account, state, form, businessDealing)
            break
          case "G642":
            getContractDetails(period, wcmJurisdiction, newForm, wcmBusiness, frm)
            break
          case "G979":
            getContractDetails(period, wcmJurisdiction, newForm, wcmBusiness, frm)
            break
   }
}

A few notes:

-The methods were built by another dev. He quit, so I am assuming his work and looking to refactor to make this better.

-The starting point is a HashMap of form numbers. I generate the HashMap, and then loop through it to gather details based on each form number in the HashMap.

-Even if I were to convert the methods into an object inheritance structure, I would still need a switch statement to know which subclass to instantiate, no? And the switch statement would look like the one above?

-Some of these case statements are calling the exact same method. Is there a way to avoid duplication?

Thanks for all your help. I'm pulling my hair out trying to figure out how to better re-engineer this. Please let me know if I can provide additional details.

CodePudding user response：

At least some cases have the same body -> use switch statement fall through

switch(form.formNumber){
      case "A1345": // fall through
      case "B254":
        getExclusionDetails(account, state, form, businessDealing)
        break;
      case "B297":
        getPartnershipDetails(account, state, form, businessDealing)
        break
      case "C397": // fall through
      case "D972": // fall through
      case "E192": // fall through
      case "E299":
        getBrokerageDetails(account, state, form, businessDealing)
        break
      case "F254": // fall through
      case "F795":
        getLocationDetails(account, state, form, businessDealing)
        break;
      case "G642": // fall through
      case "G979":
        getContractDetails(period, wcmJurisdiction, newForm, wcmBusiness, frm)
        break;
}

CodePudding user response：

You could replace the switch case with if, else if, since there are multiple conditions with the same outcome, it will reduce the repetitions.

        var formDetails = new inferForms.buildFormsMap

        for(form in formDetails){
            var formNumber = form.formNumber
            if(formNumber.equals("A1345") || formNumber.equals("A1345")){
                getExclusionDetails(account, state, form, businessDealing)
            } else if (formNumber.equals("B297") || formNumber.equals("C397")) {
                getPartnershipDetails(account, state, form, businessDealing)
            } else if (formNumber.equals("D972") || formNumber.equals("E192")) {
                getBrokerageDetails(account, state, form, businessDealing)
            } else if (formNumber.equals("F254") || formNumber.equals("F795")) {
                getLocationDetails(account, state, form, businessDealing)
            } else if (formNumber.equals("G642") || formNumber.equals("G979")) {
                getContractDetails(period, wcmJurisdiction, newForm, wcmBusiness, frm)
            }
        }