What do you think of my DNA pairing code?

Code Feedback
1

My code passes the tests but is there a better code to complete DNA strands matching challenge .Here is my code:

     function pairElement(str){
		let res =[];
		for (let char of str){
			switch(char){
			case 'A':	    
                               res.push(['A','T']);
			       break;
		        case 'T': 
                               res.push(['T','A']);
			       break;
			case 'G':
		               res.push(['G','C']);
			       break;
			case 'C':
			        res.push(['C','G']);
			        break;
				}
			}
			return res;
		}
1 Like
2

This is definitely one way to solve but as you can see there is also repeated code like your push function and your code has to evaluate every case before the next iteration. Since there are only 4 cases 1 good way to solve this in a cleaner way is to map the values into key/values in an object.

Then use a reduce function and evaluate the key in each iteration by using the [] notation for objects to evaluate each character and finally push the result to an array, it is a much cleaner solution and also objects lookup time is faster than a few cases.

function pairElement(str) {

  const pairs = {
    A: "T",
    T: "A",
    C: "G",
    G: "C"
  }

  return str.split("").reduce((acc,element)=>{
      const map = pairs[element]
      if(map){
        acc.push([element,map]);
      }

      return acc;
  },[]);

}
3

You shouldn’t use reduce if you’re just using it to do a map, it’s less clear to a reader

4

Actually looking up a hash table is much slower with only a few options. I’ll find a link in a bit.

5

?? my code here is not just mapping values , but creates a new array with paired values with 1 function call. There are alot of code examples for reduce that does this i post a link for an example below. Also using a hash table to map values and creating a new array/object is also a common problem solving technique from katas in codewars/leetcode. Roman conversion is an example a hash table is commonly used.

6

it is not just time complexity but being DRY here, if u look at the case scenario by OP u type out more code for each case and write repeated code for each case character. OP code is imperative , my code is declarative. Also for a small switch case , it evaluates every case like a if/else till there are more than 5+ cases it acts like a hash table.

7

If there are only few cases, it might not effect the speed in any case. Prefer switch if the number of cases are more than 5 otherwise, you may use if-else too.
If a switch contains more than five items, it’s implemented using a lookup table or a hash list

8

Hi sorry for slow reply, I’m well aware of what reduce does

Essentially you have a map that filters out undefineds:

str.split("").map(c => [c, pairs[c]])

The only difference is that you filter out undefined members, but that’s not necessary here really

Edit to add missing square bracket

9

fair enough, using a map directly is a lot cleaner than reduce using these test cases. But checking for undefined is a more extensible solution imo for cases where random characters are added to the function.

10

Bear in mind you can’t have random characters in this case, the nature of what this challenge actually is means that’s not possible.

11

while that is true , thinking of scenarios and thinking of extendable function help makes a better programmer. Since a lot of time we are extending and refactoring code. If this was part of a interview question and you write your own test cases it shows you know how to future proof your code

12

I would argue you are prematurely optimising for impossible conditions: anything except those four characters is corrupt input. By all means throw, but any unparseable input should not be handled otherwise – you should not, under any circumstances, filter out invalid input and carry on parsing.

1 Like
14

Premature optimization and ignoring of YAGNI principle are sure signs of a beginner programmer :slight_smile:

OP’s solution is perfectly fine - it passes test cases and it’s readable (although I would use const when declaring the array). If there would have been limitations of time/space complexity we could discuss efficiency of switch/for/reduce/hashmap etc., but as it stands now OP has solved the problem.

1 Like
15

The .reduce method is an operation that you can implement .map and .filter on, and more. This is not to say that one should necessarily always use reduce, but one should always recognize its versatility. There’s also now a .flatMap method that’s super handy.

18

Can we please keep the discussion on civil terms?

19

Thank you guys for your response