Return to Answer

Using your updated version of code.

Overall you code is bloated and noisy. Even after a clean up it feels wrong (ambiguous) as the try catch implies there are bugs, but the code gives no clue as what they may be?

If your code uses strict mode, or is part of a module (both of which should be true) it will throw a syntax error before it even runs. Reason, you are using an undeclared variable ss

Every line of code is a line that needs to maintained and understood. Each line also contains a potential bugs. Less code means code that is easier to maintain, understand, and contains fewer bugs.

• You have a lot of noisy code.

  Example rather than the 13 lines you have for

   function arrayCheck(copy, paste){
   if(Array.isArray(copy) && Array.isArray(paste)){
   return true
   } else{
   return false
   }
   }
   function stringCheck(copy, paste){
   if(String(copy) === copy && String(paste) === paste){
   return true
   } else{
   return false
   }
   }
  

  you can use 2 lines that do the same...

   const arrayCheck = (copy, paste) => Array.isArray(copy) && Array.isArray(paste);
   const stringCheck = (copy, paste) => String(copy) === copy && String(paste) === paste;
  

• Naming

  • If you are forced to use names that are very long (good names are one or two words and less than 20 characters long) use aliases to reduce the noise.

  • Names need only have semantic meaning within the scope that they exist, this scope gives names context to complete the named semantic meaning. There is no need to give every name a uniqueness across the entire code base.

  • Avoid names that have no semantic meaning, eg from you code the names ss, sa have no readable meaning, nor conforms to any commonly used abbreviations,

• Your use of switch statements is adding source noise where none is needed. Using if, else if, else statements will reduce the noise. Personally JS switch is very poorly implemented and provides none of the advantages it does in other languages. Don't use them in JS

• You have a try catch that only logs the error. This is pointless code, it also makes it harder to debug as the catch will silently hide bugs if you don't have the console open (or in this case the log is consumed by an unknown Logger).

  • Production code should not output to the console or just push throws to a log. (this is very amature).

  • Production code should not use try catch to cover unknown bugs

• Using arguments is very old school JS, also arguments has some referencing complexities that can be avoided using modern syntax.

  Use rest parameters when you have an unknown number of arguments. Eg function copyValues(...args) {

• The creating and assigning properties to options just adds noise. You don't need all the references to options.

  You can assign the named variables using destructuring assignment. eg const [copy, paste, source, target, ss_ID] = args;

• If you return an array from spreadsheetCheck you can then use a spread operator to call paste. eg stringPaste(copy, paste, ...spreadsheetCheck(

• Semicolons are required in JS, unless you know all the rules of Automatic Semicolon Insertion (ASI) ASI use semicolons.

To keep noise levels low I have used common abbreviations for source, copy, check, array, string and target (src, cpy, chk, arr, str, and trg respectively)

"use strict";
function copyValues(...args){
 const arrChk = (cpy, paste) => Array.isArray(cpy) && Array.isArray(paste);
 const strChk = (cpy, paste) => cpy instanceof String && paste instanceof String;
 const sheet = spreadsheet, sheetByName = spreadsheet.getSheetByName.bind(sheet);
 
 const argCount = args.length;
 const [cpy, paste, src, trg, ss_ID] = args;
 
 if (arrChk(cpy, paste)) {
 if (argCount === 3) {
 const ss = sheetByName(src);
 ss.getRange(paste[0], paste[1], cpy[2], cpy[3]).setValues(ss.getRange(cpy[0], cpy[1], cpy[2], cpy[3]).getValues());
 } else if (argCount === 4) {
 arrayPaste(cpy, paste, sheetByName(src), sheetByName(trg));
 } else if (argCount === 5) {
 arrayPaste(cpy, paste, ...chkSheet(src, trg, ss_ID));
 }
 } else if (strChk(cpy, paste)) {
 if (argCount === 3) {
 const ss = sheetByName(src);
 ss.getRange(paste).setValues(ss.getRange(cpy).getValues());
 } else if (argCount === 4) {
 stringPaste(cpy, paste, sheetByName(src), sheetByName(trg));
 } else if (argCount === 5) {
 stringPaste(cpy, paste, ...chkSheet(src, trg, ss_ID));
 }
 }
 function chkSheet(src, trg, ss_ID) {
 const id = SpreadsheetApp.openById(ss_ID), openByName = id.getSheetByName.bind(id);
 
 if (openByName(src) === null) {
 return [sheetByName(src), openByName(trg)];
 }
 if (openByName(trg) === null) {
 return [openByName(src), SpreadsheetApp.getActiveSpreadsheet().getSheetByName(trg)];
 }
 return [];
 }
 function arrayPaste(cpy, paste, ss, sa){
 sa.getRange(paste[0], paste[1], cpy[2], cpy[3]).setValues(ss.getRange(cpy[0], cpy[1], cpy[2], cpy[3]).getValues());
 } 
 function stringPaste(cpy, paste, ss, sa){
 sa.getRange(paste).setValues(ss.getRange(cpy).getValues());
 } 
}
```

This review pertains to the updated version of the code.

Overall the code is bloated and noisy. Even after a clean up it feels wrong (ambiguous) as the try catch implies there are bugs, but the code gives no clue as what they may be.

If your code uses strict mode, or is part of a module (both of which should be true) then it will throw a syntax error before it even runs. The reason is that it uses an undeclared variable ss.

Every line of code is a line that needs to be maintained and understood. Each line also contains potential bugs. Less code means code that is easier to maintain, understand, and contains fewer bugs.

• You have a lot of noisy code.

  Example: rather than the 13 lines you have for:

   function arrayCheck(copy, paste){
   if(Array.isArray(copy) && Array.isArray(paste)){
   return true
   } else{
   return false
   }
   }
   function stringCheck(copy, paste){
   if(String(copy) === copy && String(paste) === paste){
   return true
   } else{
   return false
   }
   }
  

  you can use 2 lines that do the same...

   const arrayCheck = (copy, paste) => Array.isArray(copy) && Array.isArray(paste);
   const stringCheck = (copy, paste) => String(copy) === copy && String(paste) === paste;
  

• Naming

  • If you are forced to use names that are very long (good names are one or two words and less than 20 characters long) then use aliases to reduce the noise.

  • Names need only have semantic meaning within the scope that they exist; this scope gives names context to complete the named semantic meaning. There is no need to give every name a uniqueness across the entire code base.

  • Avoid names that have no semantic meaning, e.g. from the code the names ss, sa have no readable meaning, nor conforms to any commonly used abbreviations.

• Your use of switch statements is adding source noise where none is needed. Using if, else if, else statements will reduce the noise. Personally JS switch is very poorly implemented and provides none of the advantages it does in other languages. Don't use them in JS

• You have a try catch that only logs the error. This is pointless code. It also makes it harder to debug as the catch will silently hide bugs if you don't have the console open (or in this case the log is consumed by an unknown Logger).

  • Production code should not output to the console or just push thrown exceptions to a log. (this is very amature).

  • Production code should not use try catch to cover unknown bugs

• Using arguments is very old school JS, also arguments has some referencing complexities that can be avoided using modern syntax.

  Use rest parameters when you have an unknown number of arguments. E.g. function copyValues(...args) {

• The creating and assigning properties to options just adds noise. You don't need all the references to options.

  You can assign the named variables using destructuring assignment. eg const [copy, paste, source, target, ss_ID] = args;

• If you return an array from spreadsheetCheck you can then use a spread operator to call paste. eg stringPaste(copy, paste, ...spreadsheetCheck(

• Semicolons are required in JS, unless you know all the rules of Automatic Semicolon Insertion (ASI) then use semicolons.

To keep noise levels low I have used common abbreviations for source, copy, check, array, string and target (i.e.src, cpy, chk, arr, str, and trg respectively)

"use strict";
function copyValues(...args){
 const arrChk = (cpy, paste) => Array.isArray(cpy) && Array.isArray(paste);
 const strChk = (cpy, paste) => cpy instanceof String && paste instanceof String;
 const sheet = spreadsheet, sheetByName = spreadsheet.getSheetByName.bind(sheet);
 
 const argCount = args.length;
 const [cpy, paste, src, trg, ss_ID] = args;
 
 if (arrChk(cpy, paste)) {
 if (argCount === 3) {
 const ss = sheetByName(src);
 ss.getRange(paste[0], paste[1], cpy[2], cpy[3]).setValues(ss.getRange(cpy[0], cpy[1], cpy[2], cpy[3]).getValues());
 } else if (argCount === 4) {
 arrayPaste(cpy, paste, sheetByName(src), sheetByName(trg));
 } else if (argCount === 5) {
 arrayPaste(cpy, paste, ...chkSheet(src, trg, ss_ID));
 }
 } else if (strChk(cpy, paste)) {
 if (argCount === 3) {
 const ss = sheetByName(src);
 ss.getRange(paste).setValues(ss.getRange(cpy).getValues());
 } else if (argCount === 4) {
 stringPaste(cpy, paste, sheetByName(src), sheetByName(trg));
 } else if (argCount === 5) {
 stringPaste(cpy, paste, ...chkSheet(src, trg, ss_ID));
 }
 }
 function chkSheet(src, trg, ss_ID) {
 const id = SpreadsheetApp.openById(ss_ID), openByName = id.getSheetByName.bind(id);
 
 if (openByName(src) === null) {
 return [sheetByName(src), openByName(trg)];
 }
 if (openByName(trg) === null) {
 return [openByName(src), SpreadsheetApp.getActiveSpreadsheet().getSheetByName(trg)];
 }
 return [];
 }
 function arrayPaste(cpy, paste, ss, sa){
 sa.getRange(paste[0], paste[1], cpy[2], cpy[3]).setValues(ss.getRange(cpy[0], cpy[1], cpy[2], cpy[3]).getValues());
 } 
 function stringPaste(cpy, paste, ss, sa){
 sa.getRange(paste).setValues(ss.getRange(cpy).getValues());
 } 
}