7
\$\begingroup\$

I am working on a CoderByte problem in JavaScript. I have solved the problem below. Although I am trying to figure out if there is an easier way to do it? I am guessing RegEx will come up, although I don't have experience with that yet.

From CoderByte:

Using the JavaScript language, have the function SimpleSymbols(str) take the str parameter being passed and determine if it is an acceptable sequence by either returning the string true or false. The str parameter will be composed of + and = symbols with several letters between them (ie. ++d+===+c++==a) and for the string to be true each letter must be surrounded by a + symbol. So the string to the left would be false. The string will not be empty and will have at least one letter.

My implementation:

var str = "++d+===+c++=";

var simpleSymbols = function (str) {
    for (var i = 0; i < str.length; i++) {
        if (str[i] != '+' && str[i] != '=' && !(str[i] >= 'a' && str[i - 1] == '+' && str[i + 1] == '+')) {
            return false;
        }
    }
    return true;
}

simpleSymbols(str);
\$\endgroup\$
  • \$\begingroup\$ regex can be very messy for anything more complicated than this. better to learn how to do things without it first, then try to do the regex version, because regex is very useful if used correctly and in the right places \$\endgroup\$ – Malachi Jan 16 '14 at 20:32
7
\$\begingroup\$

I am guessing RegEx will come up

If I understand the problem correctly, here's a possible regex solution:

function simpleSymbols(str) {
  var letters = str.match(/[a-z]/gi);
  var matches = str.match(/\+[a-z]\+/gi);
  if (! str.length || ! matches) {
    return false;
  }
  return letters.length == matches.length;
}

The function above takes a string, extracts its letters, and its matches with the pattern +letter+. If the string is empty or there are no matches, return false; otherwise check there if there are as many letters as matches, if so, then the string validates.

\$\endgroup\$
7
\$\begingroup\$

How about this logic?

  1. if the first character or last character is a letter it's false
  2. then go through the array and look for the letters
    • skip it if is a = or a +
    • if it isn't a = or a + then check that the character before and after is a +

So something like this:

var simpleSymbols = function (s) {
    if (s[0] != "+" && s[0] != "=") {
        return false;
    } else if (s[s.length - 1] != "+" && s[s.Length - 1] != "=") {
        return false;
    } else {
        for (var i = 1; i<s.Length - 1; i++) {
            if (s[i] != "+" && s[i] != "=") {
                if (s[i-1] != "+" || s[i+1] != "+") {
                    return False;
                }
            }
        }
    }
    return true;
}

This code is cleaner than what you have.

With this block of code:

  1. it's more readable
  2. it exits if the first or last character is a letter, immediately preventing unnecessary iterations in a for loop.
  3. it's easier to read
  4. it's easier to follow (in my opinion)

Bottom Line

Your code is not efficient. All of the if statements fire every iteration until the end of the string is met or a return is met.

  1. if the last character is a letter, will return false and not loop through the whole string.
  2. will not perform a "before and after check" unless the character is a letter

It may not make a difference in smaller strings, but if you have a huge string to parse, it will make a difference.


I have been told the the CoderBytes challenge will include numbers that don't need to be surrounded by +'s, and I wanted to steal a little of @Konijn's answer as well.

All of my above points still stand for this code block as well.

var simpleSymbols = function (s) {
    if (s[0] >= 'a' || s[s.length -1] >= 'a') {
        return false;
    } else {
        for (var i = 1; i<s.length - 1; i++) {
            if (s[i] >= 'a') {
                if (s[i-1] != "+" || s[i+1] != "+") {
                    return false;
                }
            }
        }
    }
    return true;
}

NOTE

This code is the most efficient way to perform this task.

  1. It exits if the first or last character is a letter because you can't have a + before the first character or a + after the last character, thus failing the filter.

  2. It only checks the before and after characters if the character is a letter. This prevents unnecessary if statements from being performed

  3. All the other answers so far will check every character and the before and after character until it returns. This could be an issue if the string is say 500,000 characters long, you would see a definite decrease in performance.

  4. This code will not loop through the whole string if there is a letter in the last character position, which would make it a lot more efficient in those cases.

\$\endgroup\$
5
\$\begingroup\$

First off, my solution was way more convoluted ;)

Other than that, 2 minor points

  • It is good practice to have a shortcut to [].length in general
  • Since 'a' is larger than '+' and '=' ( in the ASCII table ) you can drop a part of the logic

So you could try something like

var simpleSymbols = function (s) {
    for (var i = 0, length = s.length; i < length; i++) {
        if ( s[i] >= 'a' && s[i - 1] != '+' && s[i + 1] != '+' ) {
            return false;
        }
    }
    return true;
}
\$\endgroup\$
  • \$\begingroup\$ why are you doing length = s.length;? looks like extra code to me. \$\endgroup\$ – Malachi Jan 20 '14 at 8:11
  • \$\begingroup\$ @Malachi, it caches the lookup of .length, it is a common performance approach. Not sure about your other comment. \$\endgroup\$ – konijn Jan 20 '14 at 19:29

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.