Как я могу улучшить и сократить этот блок кода? [закрыто]

9

Эта функция берет строку ДНК, такую ​​как 'GTCA', и возвращает массив, содержащий правильно подобранные пары ДНК.

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;
}

Это правильно. Однако я пытаюсь найти более короткий и простой способ написать это. Может ли кто-нибудь помочь мне с тем, что я должен использовать?

CocoFlade
источник
1
используйте этот метод легко dna [i] .toLowerCase ()
Thaier Alkhateeb
9
Если ваш код работает и вам нужны улучшения, попробуйте codereview.stackexchange.com
Питер Коллингридж,
Я не думаю, что этот код работает как задумано, | не выполняет логическое ИЛИ в Javascript, как||
Ma'moun othman
2
@mamounothman - Это правда, но оба будут работать в этом коде ( ||хотя было бы лучше).
TJ Crowder

Ответы:

14

Вы можете улучшить свой код, выполнив следующие действия:

  • Когда несколько операторов if имеют одинаковую структуру, вам, вероятно, нужно использовать объект
  • Вам необходимо проверить как прописные, так и строчные буквы. Просто используйте toLowerCase()на входе.
  • Вы можете splitиспользовать строку и map()ее, а не создавать push()в ней значения массива .

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

}

Если строка может содержать любые другие конкретные письма , то вам необходимо filter()в undefinedзначениях послеmap

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

@RobG в комментариях упоминает еще одно лучшее, что мы можем удалить ненужные буквы из строки перед ее циклическим просмотром.

return dna
        .toLowerCase()
        .replace(/[^cgta]/g,'')
        .split('')
        .map(x => obj[x])
Махир Али
источник
1
Если днк содержит не перечисленный символ, у вас будут undefinedзначения в вашем окончательном массиве.
Грегори НЕЙТ
1
@ GrégoryNEUT Добавил исправление для этого случая в моем ответе
Махер Али
Или вы можете предварительно обработать строку с помощью dna.toLowerCase().replace(/[^cgta]/g,'').... ;-)
RobG
@RobG Очень понравилось. Я добавил это мой ответ.
Махер Али
1
Я пропустил, что это была строка. :-) FWIW, более удобный для Unicode способ разделения строк на массивы теперь [...dna]. Это не разбивает суррогатные пары. (Или Array.from, что особенно полезно , если вы собираетесь на карту: Array.from(dna, mappingFunction)). (Не все , что отношение здесь, я полагаю , dnaтолько содержит c, g, tи a.)
TJ Crowder
3

Я бы наверное:

  1. Используйте for-ofцикл (или, возможно, отображение с возможной фильтрацией)

  2. Используйте поисковый объект или карту

  3. При переключении / поиске сделайте строку строчной или прописной (но дублированные записи в переключателе / ​​поиске тоже работают):

Если вы знаете, что он dnaбудет когда-либо содержать только c/ C, g/ G, t/ T/ или a/ A(что, как я понимаю, верно для ДНК ;-)), то вы можете использовать Array.fromего функцию сопоставления с объектом поиска / Map:

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

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

Я использую, Array.fromпотому что он разделит строку на кодовые точки , а не только на кодовые единицы (не разбивает суррогатные пары) и имеет функцию отображения, если вы предоставляете функцию отображения. ( В основном, Array.from(str, mappingFunction)это , [...str].map(mappingFunction)но без промежуточного массива.) Вероятно , не все , что отношение здесь дается содержание вашей строки, но может иметь значение , если ваша строка может содержать суррогатные пары.

Или с 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()));
}                                                                                                                           

Если вы не можете сделать это предположение, добавьте, .filterчтобы отфильтровать те, у которых не было совпадения:

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);
}

Или, если вы хотите избежать создания дополнительного массива, который вы filterбы создали, придерживайтесь for-of(или даже вашего 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;
}
TJ Crowder
источник
2

Вы можете использовать поисковое отображение, чтобы упростить цикл:

function pairDNA(dna) {

  const pairs = [], key = { G: "GC", C: "CG", A: "AT", T: "TA" };

  for (let i = 0; i < dna.length; i ++)
    pairs.push(key[dna[i].toUpperCase()]);
  return pairs;
}
Заостренный
источник
Это интересно, я не думал делать это таким образом, спасибо!
CocoFlade
2

Может быть, не сокращен, но определенно более ремонтопригоден.

function pairDNA(dna) {
  const map = {
    C: 'CG',
    c: 'CG',
    G: 'GC',
    g: 'GC',
    T: 'TA',
    t: 'TA',
    A: 'AT',
    a: 'AT',
  };

  return dna.split('').reduce((tmp, x) => {
    if (map[x]) {
      tmp.push(map[x]);
    }

    return tmp;
  }, []);
}

Вы также можете сделать:

function pairDNA(dna) {
  const map = {
    c: 'CG',
    g: 'GC',
    t: 'TA',
    a: 'AT',
  };

  return dna.split('').reduce((tmp, x) => {
    if (map[x].toLowerCase()) {
      tmp.push(map[x]);
    }

    return tmp;
  }, []);
}
Грегори NEUT
источник
2

Вы можете попробовать использовать функцию switch caseи forEach, например, так:

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

  dna.forEach( dnaValue => {
    switch (dnaValue.toLowerCase()) {
      case "c":
        pairs.push("CG");
        break;
      case "g":
        pairs.push("GC");
        break;
      case "t":
        pairs.push("TA");
        break;
      case "a":
        pairs.push("AT");
        break;
    }
  })

  return pairs;
}
CH4B
источник
1

Вы можете использовать строчные буквы в нижнем регистре, что позволит вам удалить проверку заглавной строки:

function pairDNA(dna) {
  dna = dna.toLowerCase();
  const pairs = []
  for (let i = 0; i < dna.length; i ++) {
   if (dna[i]=== "c") {
     pairs.push("CG");
   } else if (dna[i]dna[i] === "g") {
     pairs.push("GC");
   } else if (dna[i] === "t") {
     pairs.push("TA");
   } else if (dna[i] === "a") {
     pairs.push("AT");
   }
 }

 return p;
}
Мамун
источник
1
const lookup = {
    c: "CG", 
    g: "GC", 
    t: "TA", 
    a: "AT"
};

function pairDNA(dna) {  

  const pairs = [];

  for (let i = 0; i < dna.length; i ++) {
     pairs.push( lookup[dna[i].toLowerCase()] );
  }

  return pairs;

}
Джеймс
источник