3
\$\begingroup\$

Write a function that returns a string length N of alternating strings: "A" and "B", starting with the 1st string. How could this be improved for be more performance or efficiency?

Eg: num = 5, should return "ABABA" & given num = 2, should return "AB".

function solution(num) {
	const str1 = 'A', str2 = 'B';
	let res = [];

	for(let i = 1; res.length < num; i++ ) { 
		res.push(i % 2 ? str1 : str2);
	}

  console.log(res.join(''))
}

solution(5) // return "ABABA"

\$\endgroup\$
  • \$\begingroup\$ Welcome to CodeReview. Check the help page to get tips to improve your submission. \$\endgroup\$ – aloisdg moving to codidact.com Oct 29 '19 at 16:56
  • 2
    \$\begingroup\$ Welcome to Code Review! I have rolled back your last edit. Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$ – Heslacher Oct 30 '19 at 8:36
  • \$\begingroup\$ Thanks for your feedback @Heslacher, make sense! \$\endgroup\$ – mdiiorio Oct 30 '19 at 11:45
  • 1
    \$\begingroup\$ @mdiiorio I have rolled back your two last edits for the same reason as Heslacher \$\endgroup\$ – konijn Oct 31 '19 at 10:53
5
\$\begingroup\$

A short code review;

  • Your code does not work, it should return something instead it calls console.log
  • Your for loop is icky, either update i and check i

    for(let i = 0; i < num; i++ ))

    or check res.length

    while(res.length < num)

  • solution is too generic for this function name

  • console.log() is missing a semi-colon
  • Performance;

    • Checking whether i is odd every time seems like much, you can avoid that
    • RememberString.repeat(), you could repeat by calling "AB".repeat(num/2) and add an "A" if num is odd.

Obligatory rewrite

    //repeatAB(5) => "ABABA"
    function repeatAB(n) {
        return "AB".repeat(n/2) + (n%2 ? "A" : "");
    }

    console.log(repeatAB(5)) // returns "ABABA"

I ran this approach against the other approaches for the heck of it, this approach destroys the other approaches (at least visually):

http://jsbench.github.io/#11a2b8507a7f9290c6ef6cda596f8179

enter image description here

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ @konjin my code works. I just console log the specific result and don't use a return statement to be more explicit here, as a use case. Then I checked res.length so what do you mean? I personally like avoiding using semicolons in my code, and is not a bad practice.... Thanks for your suggestion and please be more polite with the community. \$\endgroup\$ – mdiiorio Oct 30 '19 at 15:55
  • \$\begingroup\$ Greetings, your description says Write a function that returns a string length N , either your description is wrong or your code is wrong. For semicolons, you are missing only one, so you are inconsistent in applying semicolons, I assumed you prefer to put semicolons. The for comment is style, to make it easier for the reader. As for politeness, I am open to reading how you would rephrase parts or all of my answer, as I dont see impoliteness there. (though that feedback should happen on chat) \$\endgroup\$ – konijn Oct 30 '19 at 16:59
  • \$\begingroup\$ @konijn I would disagree that not adding semi colons is not bad practice. The rules for automatic semi colon insertion are not particularly simple and when working on a project with multiple developers with differing skill-sets it is unlikely that everyone will know the rules. This would then lead to an inconsistent programming approach which over time makes the code less readable. \$\endgroup\$ – Magrangs Oct 31 '19 at 9:44
  • \$\begingroup\$ @Magrangs Agreed, but not a hill to die on in this answer ;) \$\endgroup\$ – konijn Oct 31 '19 at 13:10
2
\$\begingroup\$

Why not use doubling to get a logarithmic number of steps?

    const str1 = 'A', str2 = 'B';
    let res = '';

    if (num >= 1)
        res = str1;              // 'A'
    if (num >= 2)
        res += str2;             // 'AB'

    while (2 * res.length < num) { 
        res += res;              // 'ABAB', 'ABABABAB', ...
    }
    if (res.length < num) {
        res += res.substring(0, num - res.length)
    }
    console.log(res)

EDIT

In the above code I try do create the resulting string of exactly the length required, which costs an additional conditional. We can also get rid of that at the cost of creating the string too long temporarily. The central part would then look like:

    while (res.length < num) { 
        res += res;              // 'ABAB', 'ABABABAB', ...
    }
    res = res.substring(0, num)
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ @CianPan This could be an other point of resolution, but with the function alternative we can reused it only invoking it. Thanks! \$\endgroup\$ – mdiiorio Oct 30 '19 at 11:48
  • \$\begingroup\$ @mdiiorio You're perfectly right: that's precisely "an other point of resolution" – an example of another algorithm solving the same problem. And that's it. I don't care about embedding it in a function, it's entirely the user's buisness to decide IF and HOW to incorporate this algo into the actual code (e.g. whether to return the resulting string from a function, or explicitly print it to a virtual console, or show it in some alert window, or save in some output file, etc., depending on limitations of the destination environment and predefined requirements for the solution.) \$\endgroup\$ – CiaPan Oct 30 '19 at 13:29
1
\$\begingroup\$

No need to use abbreviation when you can use meaningful name. For example, use result instead of res. Also if you can find a better name for your function, it will improve the readability. I will use alternateLetters but I think we can do better :)

To achieve your problem, you can use a for loop or a generate a range then use map to assign the letter.

function alternateLetters(num) {
    const a = 'A', b = 'B';
    const result = [...Array(num)].map((_, i) => i % 2 ? b : a);
    console.log(result.join(''))
}

alternateLetters(5) // return "ABABA"

if you prefer, you can also generate result with keys():

const result = [...Array(num).keys()].map(i => i % 2 ? b : a);

By the way I would make it more generic by passing 'A' and 'B' as parameter:

function alternateLetters(num, a, b) {
    const result = [...Array(num)].map((_, i) => i % 2 ? b : a);
    return result.join('');
}

console.log(alternateLetters(5, 'A', 'B')) // return "ABABA"

If we have more context about this problem, we may even do better. For example, are "A" and "B" always constant?

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Why did you passed the underscore on the arrow function before i ? \$\endgroup\$ – mdiiorio Oct 29 '19 at 17:53
  • 1
    \$\begingroup\$ Using underscore is a convention to show that we can ignore this value. In this case we are looking for the index. Read developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/… \$\endgroup\$ – aloisdg moving to codidact.com Oct 29 '19 at 18:17
  • \$\begingroup\$ I read and used map() before but in MDN didn't mention this specific convention. But its okey, thanks any way! \$\endgroup\$ – mdiiorio Oct 30 '19 at 11:50
  • \$\begingroup\$ @mdiiorio this is a general programming convention. Read the 3rd point here for Python or here in C# or even Go, Dart, Ruby, F#, etc. \$\endgroup\$ – aloisdg moving to codidact.com Oct 30 '19 at 13:05
  • \$\begingroup\$ thanks for clarify! I never hear about it. \$\endgroup\$ – mdiiorio Oct 30 '19 at 13:57
1
\$\begingroup\$

Optimization tips with benchmark demo tests:

Initial approach "issues":

  • for(let i = 1; res.length < num; i++ ). Instead of calculation the resulting array length res.length on each loop iteration, you can just replace it with i <= num
  • let res = []; ... res.push() ... res.join(). While the final result should eventually be a string we can declare the result holder as string and perform simple concatenation:

    let res = '';
    ...
    for (let i = 1; i <= num; i++ ) { 
        res += (i % 2 ? str1 : str2);
    }
    

The below approach is a bit more performant, in my opinion (as well as benchmarks shown that), using the mentioned improvements:

function cycle(num) {
    const str1 = 'A', str2 = 'B';
    let res = '';

    for (let i=1; i <= num; i++) { 
        res += (i % 2 ? str1 : str2);
    }
    console.log(res)
}

In prepared benchmarks I've named

  • the initial function (for loop + res array) as cycle1
  • my approach - as cycle2
  • and the function from previous answer alternateLetters - named accordingly

Shared link on http://jsbench.github.io: http://jsbench.github.io/#be710bc0a4b288e024d5bd67286b1c8c

Another benchmark were run on https://jsbench.me/ and the results are as below:

enter image description here

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Perfect approach! This is just what I was looking for. \$\endgroup\$ – mdiiorio Oct 30 '19 at 11:57
0
\$\begingroup\$

Try using this,

if n == 1:
    print "A"
elif n%2 == 0:
    temp = (n/2);
    print "AB"*temp
else:
    temp = (n-1)/2
    print "AB"*temp + "A"

In above pseudo code, you can see if n is even then print "AB" * (n/2) times and if odd reduce one number and print "AB" divided by 2 times and add "A"

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.