3
\$\begingroup\$

It took me a while to compute this script, it works perfectly, but I think that it is very raw and long.

The goal of the script is to produce a graph of cumulated rainfall with 3 lines:

  • Cumulated rainfall of the rainiest year
  • Cumulated rainfall in the current year (for example from 1st January until April)
  • Cumulated rainfall of the driest year

Do you have any suggestions on how to simplify the script?

// Cumulative function
var compute = function(day) {
  // Filter the collection from start date till the day of computatiton
  var begin1 = startMin
  var begin2 = start
  var begin3 = startMax
  var current1 = startMin.advance(day, 'day')
  var current2 = start.advance(day, 'day')
  var current3 = startMax.advance(day, 'day')
  var filtered1 = collMin.filter(ee.Filter.date(begin1, current1))
  var filtered2 = coll.filter(ee.Filter.date(begin2, current2))
  var filtered3 = collMax.filter(ee.Filter.date(begin3, current3))
  // Use sum() to calculate total rainfall in the period
  // Make sure to set the start_time for the image
  var cumulativeImage = filtered1.reduce(ee.Reducer.sum())
    .set('system:time_start', current1.millis()).rename('min')
    .addBands(filtered2.reduce(ee.Reducer.sum())
    .set('system:time_start', current2.millis()).rename('current'))
    .addBands(filtered3.reduce(ee.Reducer.sum())
    .set('system:time_start', current3.millis()).rename('max'))
  
  return cumulativeImage
}

The code is also available on Google Earth Engine.

\$\endgroup\$
1
  • \$\begingroup\$ Welcome to Code Review! I changed the title so that it describes what the code does per site goals: "State what your code does in your title, not your main concerns about it.". Please check that I haven't misrepresented your code, and correct it if I have. \$\endgroup\$ Commented Oct 10, 2021 at 13:31

2 Answers 2

1
\$\begingroup\$

Welcome to Code Review. Here are a few improvements to the code:

  • Use const and arrow notation for your functions.

  • Stick to the DRY (Don't Repeat Yourself) principle: you are doing the same logic three times.

  • beginX variables are redundant.

  • Use meaningful variable and function names.

const computeCumulativeRainfallWithMinAndMax = numDays => {
  const computeCumulativeRainfall = (numDays, startDate, data, name) => {
    const endDate = startDate.advance(numDays, 'day');
    const filteredData = data.filter(ee.Filter.date(startDate, endDate));
    return filteredData.reduce(ee.Reducer.sum())
        .set('system:time_start', endDate.millis())
        .rename(name);
  };
  const curvesData = [
    [numDays, startMin, collMin, 'min'],
    [numDays, start, coll, 'current'],
    [numDays, startMax, collMax, 'max']
  ];
  return curvesData.map(curveData => computeCumulativeRainfall(...curveData))
    .reduce((prev, curr) => prev.addBands(curr));
};
\$\endgroup\$
0
\$\begingroup\$

@Matteo, the code can be shortened by using arrays. As you can see, in your code there is repeatability and also variables ordered by numbers which can easily be indexes of an array. For example: begin1 can be begin[0], begin2 -> begin[1] and so on..

startMin, start, startMax are also repeated, so instead of writing it down over and over again, these are added in an array to be used inside the forEach() method.

And so, by using the push() methods all these arrays can be populated in a one liner.

Variables can also be declared like so: var firstVariable, secondVariable, thirdVariable; and so on. This way we don't need to repeat var in every line.

var compute = function(day) {
  var arr = [startMin, start, startMax], begin=[], current=[];
  arr.forEach(el => begin.push(el) );
  arr.forEach(el => current.push(el.advance(day, 'day')) );
  var filtered = [collMin.filter(ee.Filter.date(begin[0], current[0])), coll.filter(ee.Filter.date(begin[1], current[1])), collMax.filter(ee.Filter.date(begin[2], current[2]))],
    cumulativeImage = filtered[0].reduce(ee.Reducer.sum())
      .set('system:time_start', current[0].millis()).rename('min')
      .addBands(filtered[1].reduce(ee.Reducer.sum())
      .set('system:time_start', current[1].millis()).rename('current'))
      .addBands(filtered[2].reduce(ee.Reducer.sum())
      .set('system:time_start', current[2].millis()).rename('max'));
      
  return cumulativeImage;
}
\$\endgroup\$

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.