0

I'm trying to create a simple abstraction using Google charts, I've created a chartservice which will serve as the abstraction. The module supplies the options and data-source, and the service takes care of the rest(data is supplied by a REST API).

Here is the current code, works only for one case:

createCombo(comboBarLabels: String[], comboBarTypes: String[], options: any, element: any) {
    this.overviewService.getOverviewAggBarData().pipe(first()).subscribe(comboRequest => {
      for (const index of Object.keys(comboRequest.comboData)) {
        comboRequest.comboData[index].unshift(comboBarLabels[index]);
      }
      const data_array = [comboBarTypes, comboRequest.comboData[0],
        comboRequest.comboData[1], comboRequest.comboData[2]];
      google.charts.load('current', {'packages': ['corechart']});
      google.charts.setOnLoadCallback(() => {
        const data = ChartService.createDataTable(data_array);
        const chart = new google.visualization.ComboChart(element);
        chart.draw(data, options);
      });
    });
  }

What I want to achieve is to remove this.overviewService.getOverviewAggBarData() and replace it by a conditional function, something like this in python:

def foo(a, b):  # Adds two numbers
    return a + b
a = foo
print(a(1, 2))  # Prints 3

To make something that looks like this:

createCombo(comboBarLabels: String[], comboBarTypes: String[], options: any, element: any, source: any) {
  if (source == "OverviewAggBar"){
    get_data = this.overviewService.getOverviewAggBarData;
  } else {
    get_data = this.overviewService.getOverviewPieData;
  }
  get_data().pipe(first()).subscribe(comboRequest => {
    for (const index of Object.keys(comboRequest.comboData)) {
      comboRequest.comboData[index].unshift(comboBarLabels[index]);
    }
    const data_array = [comboBarTypes, comboRequest.comboData[0],
      comboRequest.comboData[1], comboRequest.comboData[2]];
    google.charts.load('current', {'packages': ['corechart']});
    google.charts.setOnLoadCallback(() => {
      const data = ChartService.createDataTable(data_array);
      const chart = new google.visualization.ComboChart(element);
      chart.draw(data, options);
    });
  });
}

The reason that I want to do this is because the function call is very involved, being able to abstract this part away paves the way for making an even more general function. Other solutions to achieve the same goal are more than welcome!

Solved, here's the new code:

createCombo(comboBarLabels: String[], comboBarTypes: String[], options: any, element: any, source: string) {
    let getData: any;
    if (source === 'getAggData') {
      getData = this.overviewService.getOverviewAggBarData.bind(this);
    } else {
      getData = this.overviewService.getOverviewPieData.bind(this);
    }
    getData().pipe(first()).subscribe(comboRequest => {
      const data_array = [comboBarTypes];
      for (const index of Object.keys(comboRequest.comboData)) {
        comboRequest.comboData[index].unshift(comboBarLabels[index]);
        data_array.push(comboRequest.comboData[index]);
      }
      google.charts.load('current', {'packages': ['corechart']});
      google.charts.setOnLoadCallback(() => {
        const data = ChartService.createDataTable(data_array);
        const chart = new google.visualization.ComboChart(element);
        chart.draw(data, options);
      });
    });
  }
5
  • How are getOverviewAggBarData and getOverviewPieData implemented? Do they reference this? If so, are they arrow functions? I ask because when you assign functions to a variable like this, they could potentially lose context. (Not a huge problem, just wanted to mention it). Commented Oct 9, 2018 at 16:28
  • They are super simple, like this return this.http.get<OverviewData>('http://127.0.0.1:8080/combodata'); Maybe i should just define the function inside of the other function since it's so short. Commented Oct 9, 2018 at 16:34
  • 1
    If the service methods are arrow functions you are fine. If not, you could just do something like get_data = () => this.overviewService.getOverviewAggBarData(); Commented Oct 9, 2018 at 16:40
  • Could there be more than 2 functions? If so, you could create an object and use it like a dictionary to map a string to a function. Commented Oct 9, 2018 at 16:42
  • @FrankModica Interesting suggestion, could you expand a little on that? I'm not quite sure I get it, and yes there will be more than two! Commented Oct 9, 2018 at 16:45

2 Answers 2

1

If you will have many functions, you could create a "map" from the source string to function. Then you can just add more functions to the map. Something like this:

class YourClass {
    private mapFromSourceToFunction: { [key: string]: () => Observable<YourComboResponseType> } = {
        'getAggData': () => this.overviewService.getOverviewAggBarData(),
        'getPipeData': () => this.overviewService.getOverviewPieData(),
        'getSomethingElse': () => this.overviewService.getSomethingElse()
    };

    createCombo(comboBarLabels: String[], comboBarTypes: String[], options: any, element: any, source: string) {
        let getData = this.mapFromSourceToFunction[source];

        // getData().pipe ...
    }
}
Sign up to request clarification or add additional context in comments.

2 Comments

That's fantastic, this function could get a lot cleaner and more general now!
Yep, and if the map gets huge it could even be pushed out into another class
1

If I understand you correctly I think you’re already there. JavaScript (and TypeScript) allows the same kind of behavior. What is missing in your code is declaring get_data. I’d use a ternary operator to do that:

const get_data = source === “OverviewAggBar” ? this.overviewService.getOverviewAggBarData : this.overviewService.getOverviewPieData;

3 Comments

You're right, it did work! Another issue though, since the function called references this there's a problem, any thoughts on solutions for that?
You would use bind on the methods, like this.overviewService.getOverviewAggBarData.bind(this)
Wouldn't that bind the service to the current context? I'd think you'd have to do something like this.overviewService.getOverviewAggBarData.bind(this.overviewService)

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.