HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

A1Notation conversion to row & column index

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
conversioncolumna1notationindexrow

Problem

This set of Google Apps Script helper functions are published in this gist, along with a bunch of other utilities. Background info is in this SO question. Since no Google services are used, this is pure JavaScript.

What I hope to get from a code review:

  • General feedback on clarity & maintainability.



  • Is the regex in cellA1ToIndex() too big a drag on performance? What alternatives would be as-or-more functional, with better performance?



cellA1ToIndex( string cellA1, number index )

Convert a cell reference from A1Notation to 0-based indices (for arrays) or 1-based indices (for Spreadsheet Service methods).

colA1ToIndex( colA1, index )

Return a 0-based array index corresponding to a spreadsheet column label, as in A1 notation.

rowA1ToIndex( string rowA1, number index )

Return a 0-based array index corresponding to a spreadsheet row number, as in A1 notation. Almost pointless, really, but maintains symmetry with colA1ToIndex().

```
/**
* Convert a cell reference from A1Notation to 0-based indices (for arrays)
* or 1-based indices (for Spreadsheet Service methods).
*
* @param {String} cellA1 Cell reference to be converted.
* @param {Number} index (optional, default 0) Indicate 0 or 1 indexing
*
* @return {object} {row,col}, both 0-based array indices.
*
* @throws Error if invalid parameter
*/
function cellA1ToIndex( cellA1, index ) {
// Ensure index is (default) 0 or 1, no other values accepted.
index = index || 0;
index = (index == 0) ? 0 : 1;

// Use regex match to find column & row references.
// Must start with letters, end with numbers.
// This regex still allows induhviduals to provide illegal strings like "AB.#%123"
var match = cellA1.match(/(^[A-Z]+)|([0-9]+$)/gm);

if (match.length != 2) throw new Error( "Invalid cell reference" );

var colA1 = match[0];
var rowA1 = match[1];

return { row: rowA1ToIndex( rowA1, index ),
col: colA1ToIndex( colA1, index )

Solution

I see you fixed the index-checking/defaulting bug that gengkev pointed out in the comments. As SpiderPig mentioned the cleanest solution is probably index = index ? 1 : 0.

However, I don't know if I'd bother with the index parameter at all. So far, it's led to bugs, duplication across all three functions, and all it really does is add/subtract 1. That can be done anywhere.

So I'd recommend just translating to 1-based indices, and if you need zero-based indices, well, subtract 1.

I wouldn't worry about using a regex - but I'd probably make it stricter. I imagine that a cell reference isn't that flexible. I imagine the string may contain "$"s but that's about it. Correct me if I'm wrong.

However, cellA1ToIndex() also does too much. It checks things that the other two functions don't. So if you call the other two functions directly, your input isn't checked as strictly. For instance, colA1ToIndex() will accept a string like "%#", and return something wrong. Similarly, rowA1ToIndex() will gladly accept just about anything, and return NaN.

So I'd keep cellA1ToIndex as "dumb" as possible, and make sure the specialized functions are more robust.

Lastly, why a of maximum 2 letters for columns? Why not 3? Or 4?

I'd do something like this:

function cellA1ToIndex(cellA1) {
  var match = cellA1.match(/^\$?([A-Z]+)\$?(\d+)$/);

  if(!match) {
    throw new Error( "Invalid cell reference" );
  }

  return {
    row: rowA1ToIndex(match[2]),
    col: colA1ToIndex(match[1])
  };
}

function colA1ToIndex(colA1) {
  var i, l, chr,
      sum = 0,
      A = "A".charCodeAt(0),
      radix = "Z".charCodeAt(0) - A + 1;

  if(typeof colA1 !== 'string' || !/^[A-Z]+$/.test(colA1)) {
    throw new Error("Expected column label");
  }

  for(i = 0, l = colA1.length ; i < l ; i++) {
    chr = colA1.charCodeAt(i);
    sum = sum * radix + chr - A + 1
  }

  return sum;
}

function rowA1ToIndex(rowA1) {
  var index = parseInt(rowA1, 10)
  if(isNaN(index)) {
    throw new Error("Expected row number");
  }
  return index;
}

Code Snippets

function cellA1ToIndex(cellA1) {
  var match = cellA1.match(/^\$?([A-Z]+)\$?(\d+)$/);

  if(!match) {
    throw new Error( "Invalid cell reference" );
  }

  return {
    row: rowA1ToIndex(match[2]),
    col: colA1ToIndex(match[1])
  };
}

function colA1ToIndex(colA1) {
  var i, l, chr,
      sum = 0,
      A = "A".charCodeAt(0),
      radix = "Z".charCodeAt(0) - A + 1;

  if(typeof colA1 !== 'string' || !/^[A-Z]+$/.test(colA1)) {
    throw new Error("Expected column label");
  }

  for(i = 0, l = colA1.length ; i < l ; i++) {
    chr = colA1.charCodeAt(i);
    sum = sum * radix + chr - A + 1
  }

  return sum;
}

function rowA1ToIndex(rowA1) {
  var index = parseInt(rowA1, 10)
  if(isNaN(index)) {
    throw new Error("Expected row number");
  }
  return index;
}

Context

StackExchange Code Review Q#90112, answer score: 4

Revisions (0)

No revisions yet.