CODEWITHSUNDEEP
pythonpython-3.x
KcH
KcH

Reputation: 3502

how can i reduce the lines of code which are under different conditions

snippet is as below:

@api.route('/<graphType>/<fieldType>')
def display1(graphType,fieldType):
    dbconnection = dbconnector.configuration('farmautomation')
    if graphType == "line" and fieldType == "weather details":
        dataframe_name = DataFrame_Conversion.weather_details_dataFrame(dbconnection)
        groupbyvalue = 'village'
        fieldstoaggregate = 'humidity'
        datamin, datamax = Groupby.groupby(dataframe_name, groupbyvalue, fieldstoaggregate,
                                           field='humidity')
        return Graphs.weather(datamin, datamax)

    elif graphType == "pie" and fieldType == "total farmers":
        dataframe_name = DataFrame_Conversion.logins_time_dataFrame(dbconnection)
        groupbyvalue = 'created_time'
        fieldstoaggregate = 'count'
        result = Groupby.groupby(dataframe_name, groupbyvalue, fieldstoaggregate,
                             field='logins')
        return Graphs.pie(result)
    elif graphType == "bar" and fieldType == "logins":
        dataframe_name = DataFrame_Conversion.logins_time_dataFrame(dbconnection)
        groupbyvalue = 'created_time'
        fieldstoaggregate = 'count'
        result = Groupby.groupby(dataframe_name, groupbyvalue, fieldstoaggregate,
                                 field='logins')
        return Graphs.bar(result)

The code inside both the condition's(elif's) are same but have different return type, is there a possibility to reduce my code?

I have got no idea to try someway, any help is appreciated

Upvotes: 1

Views: 84

Answers (1)

meshtron
meshtron

Reputation: 1224

Looks like the 2nd and 3rd conditions are essentially the same, so they could be combined as such:

@api.route('/<graphType>/<fieldType>')
def display1(graphType,fieldType):
    dbconnection = dbconnector.configuration('farmautomation')
    if graphType == "line" and fieldType == "weather details":
        dataframe_name = DataFrame_Conversion.weather_details_dataFrame(dbconnection)
        groupbyvalue = 'village'
        fieldstoaggregate = 'humidity'
        datamin, datamax = Groupby.groupby(dataframe_name, groupbyvalue, fieldstoaggregate,
                                           field='humidity')
        return Graphs.weather(datamin, datamax)

    elif graphType in ['pie','bar'] and fieldType in ['logins','total farmers']:
        dataframe_name = DataFrame_Conversion.logins_time_dataFrame(dbconnection)
        groupbyvalue = 'created_time'
        fieldstoaggregate = 'count'
        result = Groupby.groupby(dataframe_name, groupbyvalue, fieldstoaggregate,
                             field='logins')
        if graphType == 'pie':
            return Graphs.pie(result)
        else:
            return Graphs.bar(result)

There might be a better way yet to refactor, but it depends on if you have more (lots of) cases to test or just these few.

EDIT1: Fixed total farmers string

EDIT2: Based on additional comment/question about lots of paths, added another structure that might work.

Disclaimer: I'm relatively new to Python and this code has not been tested, but the general idea is to consolidate the unbounded parts of your problem to a single spot - in this case a Dictionary - so that you have a single source of truth for that part.

@api.route('/<graphType>/<fieldType>')
def display1(graphType,fieldType):
    # Open your db connection
    dbconnection = dbconnector.configuration('farmautomation')
    # Get your chart data based on selection
    chart_args = get_aggregate_data(fieldType, dbconnection)
    # Try to get a chart with that data
    chart = get_chart(graphType, chart_args)

    if chart != None:
      return chart
    else:
      # Throw an error, return None - your callable
      pass


class AggregationSettings:
  def __init__(self, df_name='', group_by='',
       fields_to_agg='', over_field=''):
    self.df_name = df_name
    self.group_by = group_by
    self.fields_to_agg = fields_to_agg
    self.over_field = over_field

# This dictionary is what will grow as you add routes
DATAFRAME_TRANSLATE = {
  "weather details":AggregationSettings("weather_details","village","humidity","humidity"),
  "logins":AggregationSettings("logins_time","created_time","count","logins"),
  "total farmers":AggregationSettings("logins_time","created_time","count","logins")
}

def get_aggregate_data(fieldType, db_connection):
  agg_setting = DATAFRAME_TRANSLATE.get(fieldType, None)
  if agg_setting == None:
    # not in the dict
    raise ValueError('The specified fieldType is not valid')

  df_val = agg_setting.df_name + '_dataFrame'
  # NOTE: I'm not familiar with pandas (which I think this is), but I 
  # suspect there is a way for you to get your return value passing the
  # name, not the class explicitly.  If not, there are ways around that too
  dataframe_name = DataFrame_Conversion.get_by_name(df_val, db_connection)
  return Groupby.groupby(dataframe_name, agg_setting.group_by,
                        agg_setting.fields_to_agg, agg_setting.over_field)

def get_chart(graphType, chart_args):
  # I expect the number of charts is small(ish) and bounded,
  # so just use if/else hex
  if graphType == 'pie':
    return Graphs.pie(chart_args)
  elif graphType == 'bar':
    return Graphs.bar(chart_args)
  else:
    # handle other cases as needed...
    pass

  raise ValueError('The specified graphType is not valid')

Upvotes: 2

Related Questions