CODEWITHSUNDEEP
javascriptreactjs
Alwaysblue
Alwaysblue

Reputation: 11880

Is it a bad practise to not return anything from js function?

I have been using react for a while and I have observed that I don't usually return anything from function rather just call function, change global variables and so on..

For example consider my index.js file in react app

Here I have this function which formats data for the graph 

 graphDataFormating = () => {  
      let m = 0
      let arrayPush = []
      let data = this.state.data["groups"]
      let Datalength = this.state.data["groups"].length  
      this.graphdata = []
        if (!this.state.optionalTask)   Datalength = 1
        for (let i=0; i<Datalength; i++) {
            for (let j=0; j<data[i]["peaks"].length; j++) {
                  arrayPush = []
                for (let k=0; k<data[i]["peaks"][j]["eic"]["rt"].length; k++) {
                    if (this.state.filterIntensity < data[i].peaks[j]["eic"]["intensity"][k] && this.state.filterRt < data[i].peaks[j]["eic"]["rt"][k]) {
                        arrayPush.push({
                            y: (data[i]["peaks"][j]["eic"]["intensity"][k]/1000),
                            x: data[i]["peaks"][j]["eic"]["rt"][k]
                        })
                    }
                }
                this.graphdata[m] = arrayPush
                m++
            }
      }

      this.setState({graphData: this.graphdata})
    }

Here, I am not returning anything rather changing my global variables (like this.graphdata[m]) or state this.setState({graphData: this.graphdata})

Now, I call this function multiple times in my programme 

  userFilterData = (event) => {
    this.setState({[event.target.name]: event.target.value}, () => {
        this.graphDataFormating()
    })
  }


  resetData = () => {
    this.setState({  filterIntensity: -1, filterRt: -1}, () => {
        this.graphDataFormating()
    })
  }

  changetask = () => {
      this.setState({ optionalTask: !this.state.optionalTask}, () => {
        this.graphDataFormating()
      })

but all the functions, I have created including the graphDataFormating(), I haven't returned anything even for once.

and since I guess if we don't return anything from js function, it will automatically return undefined? hence I believe all the functions above are returning undefined.

With this, I have two questions,

Is it a bad practise to not return anything from the function?

If you were to improve this code, what suggestion would you give?

Upvotes: 0

Views: 872

Answers (5)

Morgan
Morgan

Reputation: 71

I know this post is old, but I came across it today and thought I'd add.

It's not wrong, but there's benefits in doing it the other way.

If you refactored your code to pass in and return like this:

getFormattedData = (data, filterIntensity, filterRt) => {
  let m = 0;
  let arrayPush = [];
  let data = data['groups'];
  let Datalength = data['groups'].length;
  const graphdata = [];
  if (!this.state.optionalTask) Datalength = 1;
  for (let i = 0; i < Datalength; i++) {
    for (let j = 0; j < data[i]["peaks"].length; j++) {
      arrayPush = [];
      for (let k = 0; k < data[i]["peaks"][j]["eic"]["rt"].length; k++) {
        if (filterIntensity < data[i].peaks[j]['eic']['intensity'][k] && filterRt < data[i].peaks[j]['eic']['rt'][k]) {
          arrayPush.push({
            y: (data[i]['peaks'][j]['eic']['intensity'][k] / 1000),
            x: data[i]['peaks'][j]['eic']['rt'][k],
          });
        }
      }
      graphdata[m] = arrayPush;
      m++;
    }
  }

  return graphdata;
};

You could then test this function without your state being touched, and with as many options as you like.

It also seems like then you maybe don't even need to set the filterIntensity and filterRt states.

resetData = () => {
  this.setState({
    graphdata: this.getFormattedData(
      this.state.data,
      -1,
      -1,
    ),
  })
}

Upvotes: 0

ingdc
ingdc

Reputation: 893

As per specifications, "A function returns undefined if a value was not returned." To answer your questions:

1) "Is it a bad practice to not return anything from the function?" No. A function returning undefined can be thought of as a "procedure" in other languages, i.e. a subroutine that may have some side effects but doesn't return any value.

2) "If you were to improve this code, what suggestion would you give?" Nothing related to functions returning undefined.

Upvotes: 0

zfrisch
zfrisch

Reputation: 8670

Yes and No

Truthfully it depends.

If you're utilizing methods to alter your state in a state-based application, there's nothing wrong with not returning anything from a function. The priority in that scenario is on completing an action rather than the manipulation of data. You're basically saying "Hey Application, this has changed! Rerender!"

If, however, you find yourself manipulating data (an array or an object) that is not part of state, and by virtue of that not reporting an action, it is best to follow the functional programming paradigm, and instead of mutating that data, to create a new entity of the same data type, populate it, and return it.

Upvotes: 3

justin.m.chase
justin.m.chase

Reputation: 13675

No, its fine.

The only suggestion would be to try to use .map on arrays instead of a triple for loop and push, but what you have looks pretty much fine to be honest.

Upvotes: 1

Bruinen
Bruinen

Reputation: 109

As you asked what can you improve in that code I'd suggest to replace for (let j=0; j<data[i]["peaks"].length; j++) with for (let j of data[i]["peaks"]) I think it's nicer form of looping through array.

Upvotes: 1

Related Questions