OSAMA E
OSAMA E

Reputation: 341

How can I reduce complexity of this switch case?

I have the following one case in switch case which contains if else if statement. How can I reduce its complexity? Is there any concise way I can refactor this code to reduce 3 points to 1?

case 'glidepath':             ------->  +1 

   if (chartOptions.type === trp.fai.charts.glidepath.types.OVERVIEW) { --->+1
     trp.fai.charts.glidepath.drawOverview(chartOptions.data, $chart.attr('id'));   } 

else if (chartOptions.type === trp.fai.charts.glidepath.types.DETAILS) { -->+1
      trp.fai.charts.glidepath.drawDetails(chartOptions.data, $chart.attr('id'));

      break;
    }

Upvotes: 1

Views: 220

Answers (3)

Ortiga
Ortiga

Reputation: 8824

What about using convention based on your variables instead of a direct call:

You could create

chartHandlers: {
    glidepathDETAILS: function(data, id){ drawDetails(data, id); },

    glidepathOVERVIEW: function(data, id){  drawOverview(data, id); }

    invoke: function(switchKey, type, data, id){ this[switchKey + type.ToString()].apply(data, type); }
}

Notice how invoke function calls your function based on the names of your switch key and chart type.

Then you could replace your entire switch to a single call.

chartHandlers.invoke(switchKey, chartOptions.type, chartOptions.data, $chart.attr('id'));

Disclaimer: I haven't programmed in JS in a while and haven't checked my syntax. This is just to give you an idea. Sorry for possible silly mistakes.

Upvotes: 0

brandt.codes
brandt.codes

Reputation: 913

Split it in multiple functions.

case 'glidepath':
  drawTypes(type);
  break;
}
  // --- END FUNCTION ---------------


function drawTypes(type) {
  const {type, data} = chartOptions;
  const chart = $chart.attr('id');    
  cont {glidepath: {types,drawOverview},OVERVIEW,DETAILS} = trp.fai.charts;
  if (type === types.DETAILS) { // Put DETAILS first to break out earlier
      drawOverview(data, chart);
  } 
  else if (type === types.OVERVIEW) {
    drawDetails(data, chart);
  }
}

Upvotes: 1

Charlie Martin
Charlie Martin

Reputation: 8406

case 'glidepath':      
  const { types, drawOverview, drawDetails } = trp.fai.charts.glidepath
  const id = $chart.attr('id')

  switch(chartOptions.type) {
    case types.OVERVIEW:
      drawOverview(chartOptions.data, id); 
      break;
    case types.DETAILS:
      drawDetails(chartOptions.data, id);
      break;
  }

EDIT: Based on your comment, the goal here is to remove if, if else and case statements. I don't think this improves the code in any way, but you could do this...

case 'glidepath':      
  const { types, drawOverview, drawDetails } = trp.fai.charts.glidepath
  const id = $chart.attr('id')
  const actions = {
    [types.OVERVIEW]: drawOverview,
    [types.DETAILS]: drawDetails
  }
  actions[chartOptions.type](chartOptions.data, id)

Upvotes: 0

Related Questions