Seeking help for spreadsheet script for making it compact (date recovery from serialnumbers script)

Question

I wrote some working code in spreadsheet scripts, but it's a little bulky and slow. Is there any way to refactor this to be more compact and faster?

I'm a beginner, so I'm happy it works at the moment. So no worries if you can't help or it involves too long to give a solution. I would love to learn.

function extract() {
  var ss = SpreadsheetApp.getActive();
  var sh = ss.getActiveSheet();
  var dt = ss.getDataRange().getValues();
  var res = new Array();

  for (var i = 1; i < dt.length; i++) {
    if (dt[i][3].toString().charAt(0) == "E") {
      res.push([dt[i][5], dt[i][6]])
    } else if ((dt[i][5] != null && dt[i][5] != "") || (dt[i][6] != null && dt[i][6] != "")) {
      res.push([dt[i][5], dt[i][6]])
    } else if (dt[i][3].toString().trim() != null && dt[i][3].toString().trim() != "") {
      if (dt[i][2] == "DMM" || dt[i][2] == "Rock Exotica" || dt[i][2] == "Lyon") {
        var tt = methd1(dt[i][3])
      } else if (dt[i][2] == "Petzl" || dt[i][2] == "Marlow") {
        var tt = methd2(dt[i][3])
      } else if (dt[i][2] == "CMC") {
        var tt = methd3(dt[i][3])
      } else if (dt[i][2] == "Singing Rock") {
        var tt = methd4(dt[i][3])
      } else if (dt[i][2] == "Taz") {
        var tt = methd5(dt[i][3])
      } else {
        tt = [null, null]
      }
      res.push(tt)
    } else {
      res.push([null, null])
    }
  }

  sh.getRange(2, 6, res.length, res[0].length).setValues(res)
}

function methd5(vl) {
  var dte = new Date();
  var yr;
  var mnth;

  mtch = vl.toString().match(/(?<=^...)\d{4}/gm)

  if (mtch != null) {
    mtch = mtch[0]
    yr = parseInt(mtch.substring(0, 2))
    
    if (yr <= (dte).getFullYear() - 2000) {
      yr = yr + 2000;
    } else {
      yr = yr + 1900;
    }
    
    mnth = parseInt(mtch.substring(2, ))
  }

  return ([yr, mnth])
}


function methd4(vl) {
  var dte = new Date();
  var yr;
  var mnth;

  mtch = vl.toString().match(/\d{4}$/gm)

  if (mtch != null) {
    mtch = mtch[0]
    yr = parseInt(mtch.substring(2, ))
    if (yr <= (dte).getFullYear() - 2000) {
      yr = yr + 2000;
    } else {
      yr = yr + 1900;
    }

    mnth = parseInt(mtch.substring(0, 2))
  }

  return ([yr, mnth])
}

function methd3(vl) {
  var dte = new Date();
  var yr;
  var mnth;

  mtch = vl.toString().match(/^\d\d\d\d\d\d\d/gm)

  if (mtch != null) {
    mtch = mtch[0]
    yr = parseInt(mtch.toString().substring(0, 4))
    mnth = (new Date((new Date(dte.getFullYear(), 0, 1)).getTime() + parseInt((mtch.substring(2, ))) * 24 * 60 * 60 * 1000)).getMonth() + 1
  }

return ([yr, mnth])
}

function methd2(vl) {
  var dte = new Date();
  var yr;
  var mnth;

  var rr = [null, null]
  var lst = ["A", "B", "C", "D", "E", "F", "G", "H", "I", "J", "K", "L"]
  var mtch = vl.toString().match(/^\d\d[A-Z]/gm)
  if (mtch != null) {
    mtch = mtch[0]
    yr = parseInt(mtch.substring(0, 2))

    if (yr <= (dte).getFullYear() - 2000) {
      yr = yr + 2000;
    } else {
      yr = yr + 1900;
    }

    mnth = lst.indexOf(mtch.substring(2, )) + 1
    rr = [yr, mnth]
  } else {
    mtch = vl.match(/^\d\d\d\d\d/gm)

    if (mtch != null) {
      mtch = mtch[0]

      yr = parseInt(mtch.toString().substring(0, 2))
      if (yr <= (dte).getFullYear() - 2000) {
        yr = yr + 2000;
      } else {
        yr = yr + 1900;
      }

      mnth = (new Date(((new Date(dte.getFullYear(), 0, 1)).getTime() + parseInt((mtch.substring(2, ))) * 24 * 60 * 60 * 1000))).getMonth() + 1
      rr = [yr, mnth]
    }
  }

  return (rr)
}

function methd1(vl) {
  var dte = new Date();
  var yr;
  var mnth;

  mtch = vl.match(/^\d\d\d\d\d/gm)

  if (mtch != null) {
    mtch = mtch[0]
    yr = parseInt(mtch.toString().substring(0, 2))

    if (yr <= (dte).getFullYear() - 2000) {
      yr = yr + 2000;
    } else {
      yr = yr + 1900;
    }

    mnth = (new Date((new Date(dte.getFullYear(), 0, 1)).getTime() + parseInt((mtch.substring(2, ))) * 24 * 60 * 60 * 1000)).getMonth() + 1
  }

  return ([yr, mnth])
}

Answer 1

To be honest, this code is very difficult to review because of all the abbreviated identifiers. In general, do not abbreviate things. Give functions and variables names that are meaningful to your application. It is nigh impossible to suggest performance improvements until identifiers are named properly.

Some other structural suggestions:

• Use array literal syntax [] instead of new Array() when initializing an empty array. The array literal is idiomatic, and there is no advantage of using the new operator.

• Eliminate redundant variable declarations in the extract function. The body of the for loop declares the tt variable multiple times. Worse yet, the else block omits the var directive, which creates the tt variable as an implicit global variable. Declare the tt variable at the top of the block in which it will be used prior to the if-else construct.

• No need to surround date objects with parenthesis. I see a lot of (dte).foo() calls. Use dte.foo() instead. The parenthesis just obfuscates the code.

• Always specify a radix with parseInt. The radix informs parseInt how to interpret the string. Is it a binary number, hexadecimal, base 10? It makes a difference.

  parseInt(mtch.substring(2, ), 10)
  //                          ^^^^
  

• No need to surround function return values with parenthesis. The methd5 function was the first one that stood out to me.

  • Hard to read: return ([yr, mnth])

  • Better: return [yr, mnth];

  • Best: return [year, month]; (which I note in the opening paragraph of my answer).

    There is only a single character difference between mnth and month, but the word spelled out is much easier to read. Abbreviations cause people to focus on what the abbreviation means rather than the bigger picture of what the program is doing. This is a massive distraction to any programmer.

• Simplify regular expressions with repeated character patterns: /^\d\d\d\d\d\d\d/gm should be /^\d{7}/gm. This is all the more confusing because several lines up I saw /\d{4}/, so I'm not sure why the same repetition token wasn't used elsewhere.

• I have no idea what this is, but it appears twice:

  mnth = (new Date(((new Date(dte.getFullYear(), 0, 1)).getTime() + parseInt((mtch.substring(2, ))) * 24 * 60 * 60 * 1000))).getMonth() + 1
  

  Even after naming variables better, I would have a hard time understanding what this code is doing. Worse yet, it appears copied and pasted. Move this into a function with a good name. Sometimes things are messy and there is no good way to clean them up. It helps to hide this messiness behind a good function or variable name.

I'm not familiar with the Sheets API, but I keep seeing this vl variable (short for "value"?). The toString() method is called repeatedly on this variable. If it is already a String then there is no need to call toString(), but maybe it's an object of a different type which makes the call to toString() necessary.

Despite all the items I noted above, none of the functions have a large number of lines of code — this is a good thing. The biggest overall improvement I can suggest is naming things properly. I think all other improvements will follow that.