How can I improve and shorten this block of code? [closed]

Question

This function takes a string of DNA such as 'GTCA' and returns an array containing correctly matched DNA pairs.

function pairDNA(dna) {

  const pairs = []

  for (let i = 0; i < dna.length; i ++) {

    if (dna[i] === "C" | dna[i] === "c") {
      pairs.push("CG");
    } else if (dna[i] === "G"| dna[i] === "g") {
      pairs.push("GC");
    } else if (dna[i] === "T"| dna[i] === "t") {
    pairs.push("TA");
    } else if (dna[i] === "A"| dna[i] === "a") {
      pairs.push("AT");
    }
  }

return pairs;
}

This is correct. However i'm trying to find a shorter, simpler way of writing it. Can anyone help me with what I should be using?

Answer 1

You can improve you code in following steps:

• When there are multiple if statements and all have same structure you probably need to use an object
• You need to check of both uppercase and lowercase. Just use toLowerCase() on input.
• You can split the string and map() it rather than creating an array push() values into it.

function pairDNA(dna) {
  const obj = {
    c: 'CG',
    g: 'GC',
    t: 'TA',
    a: "AT"
  }
  return dna.split('').map(x => obj[x.toLowerCase()])

}

If the string could contain anything other the specific letters then you need to filter() the undefined values after map

return dna.split('').map(x => obj[x.toLowerCase()]).filter(x => x !== undefined)

Another better is mentioned by @RobG in the comments that we can remove the unwanted letters from string before looping through it.

return dna
        .toLowerCase()
        .replace(/[^cgta]/g,'')
        .split('')
        .map(x => obj[x])

Answer 2

I'd probably:

1. Use a for-of loop (or possibly mapping with possible filtering)

2. Use a lookup object or Map

3. Make the string lower or upper case when switching/looking up (but duplicated entries in the switch/lookup work too):

If you know that dna will only ever contain c/C, g/G, t/T/, or a/A (which, as I understand it, is true of DNA ;-) ), then you can use Array.from with its mapping feature with a lookup object/Map:

const table = {
    c: "CG",
    g: "GC",
    t: "TA",
    a: "AT"
};

function pairDNA(dna) {
  return Array.from(dna, entry => table[entry.toLowerCase()]);
}                                                                                                                           

I'm using Array.from because it will split the string on code points, not just code units (doesn't break up surrogate pairs) and has a mapping feature if you provide a mapping function. (Basically, Array.from(str, mappingFunction) is [...str].map(mappingFunction) but without the intermediate array.) Probably not all that relevant here given the content of your string, but can matter if your string may contain surrogate pairs.

Or with a Map:

const table = new Map([
  [c, "CG"],
  [g, "GC"],
  [t, "TA"],
  [a, "AT"]
]);

function pairDNA(dna) {
  return Array.from(dna, entry => table.get(entry.toLowerCase()));
}                                                                                                                           

If you can't make that assumption, add .filter to filter out the ones that didn't have a match:

function pairDNA(dna) {
  return Array.from(dna, entry => table.get(entry.toLowerCase())).filter(Boolean);
  // or if using an object: return dna.map(entry => table[entry.toLowerCase()]).filter(Boolean);
}

Or if you want to avoid creating the extra array the filter would create, stick with for-of (or even your for):

const table = {
    c: "CG",
    g: "GC",
    t: "TA",
    a: "AT"
};

function pairDNA(dna) {
  const pairs = [];

  for (const entry of dna) {
    const value = table[entry.toLowerCase()];
    if (value) {
      pairs.push(value);
    }
  }
  return pairs;
}