6
\$\begingroup\$

In a recent interview I was asked to solve the below problem.

Problem:

Given a string with variables in them, e.g.
"I am a string with {{ variable }} in them"

Create an output where the {{ variable}} is replaced with their actual values.

So if given the input:

var template = {
  "sentence": "The {{ object }} {{ verb }} ",
  "object": "{{ adjective }} {{ noun }}",
  "verb": "flew",
  "adjective": "colorful",
  "noun": "bird"
};

the function call

fillInTheBlanks(template, 'sentence')

must produce the output:

The colorful bird flew

I came up with the following solution (which works):

var sentence = fillInTheBlanks(template, 'sentence');

//Accomplish however you'd like, additional utility functions are fine
function fillInTheBlanks(template, outputKey) {

  var val = template[outputKey];

  while (val.search(/{{/i) >=0) {
      val = splitAndMerge(val);
      val = val.replace(/,/g, " ");
  }

 return val;
}

function splitAndMerge(val) {
   var splittedSentence = val.split(' ');
    _.forEach(splittedSentence, function(item, idx) {
    if (item === '{{') {
      splittedSentence[idx+1] = resolveString(splittedSentence[idx+1]);
      splittedSentence.splice(idx+2, 1);
      splittedSentence.splice(idx, 1);
    }
  });

  return  splittedSentence.toString();

}


function resolveString(item) {
  return template[item];
}

PLNKR: http://plnkr.co/edit/4ciGzVewZRDi4RcSDsfI?p=preview

However, this solution was not considered elegant. what would have been a better way to solve this ?

\$\endgroup\$
  • 1
    \$\begingroup\$ @Flambino's answer is the best approach here. As a side note, you shouldn't use underscore when vanilla js will do. forEach is available on the Array class. \$\endgroup\$ – Jonah Nov 11 '15 at 20:36
4
\$\begingroup\$

Generally, using split and treating a string as an array - only to turn it back into a string - isn't very elegant for find-and-replace task like this. If the input is a string, and the output is a string, making the intermediate step an array seems odd given that JS has some OK string manipulation features.

Your code is also buggy. Since you use toString instead of a plain join, you introduce commas in the string. And you then have to remove those again, but you do that with a global replace. Meaning the string can't have commas of its own, or they'll get eaten too.

Instead of all that, you could simply do join(" "), and get the same result as you would with toString() & replace(/,/g, " ") but without stomping on existing commas.

You're also replying on spaces in the string. But really, the only delimiters you should care about are {{ and }}. I should be able to write a template string like "{{ foo }}{{ bar }}" (no space in between) to get a string like StackOverflow. Or I might use hyphens or something else (e.g. the aforementioned commas) and I wouldn't expect the code to eat them. I would also expect to be able to use two spaces inside the braces - or no spaces at all. Neither thing is handled properly by your code, as it's totally dependent on the whitespace being just so.

Lastly, your code would fail on malformed template strings like "here are two braces {{ and here two more {{". That's not a proper template, and should just be returned unaltered. But your code only looks for the opening braces and assumes there to be two close braces later on. So given the input above, I get "here are two braces two more " as output, because it aggressively splices away some words.

A couple of minor notes:

  • /{{/i - the i flag (case-insensitive matching) doesn't matter here. There is no upper- or lowercase for curly braces.

  • "Split" as an adjective is also "split". So "splitted sentence" isn't quite grammatically kosher.

  • You could combine the splice calls, since splice both removes and inserts elements in an array. I.e.

    splittedSentence.splice(idx, 3, resolveString(splittedSentence[idx+1]));
    
  • The resolveString() function is problematic. For one, it's a bit much to wrap property access in a function. In fact, calling the function is longer than just writing template[item]. But more importantly the function relies on the template object being accessible in its scope. Note that fillInTheBlanks, the point of the task, does not make such an assumption; there you must pass in the object. Which means you could call fillInTheBlanks("x", { x: "hi!" }) and it should work. But in your case it would fail, because the template object won't be in resolveString()'s scope.

  • And as @Jonah pointed out, and which I'd missed, you're using underscore (or lodash) just to do a forEach, when forEach is built in. Sure, some really old browsers lack it, but if that's a concern there's always the plain old for-loop.

Anyway, I said you don't need to split the string as there are string manipulation functions you can use instead. Specifically, the String.replace method accepts a function as its 2nd argument. That function will be passed the matched part(s) of the string, and its return value will be used as the replacement. Add in a regular expression, and you've got a pretty neat solution.

So I imagine that your interviewers were looking for something like this perhaps:

var template = {
  "sentence": "The {{ object }} {{ verb }} ",
  "object": "{{ adjective }} {{ noun }}",
  "verb": "flew",
  "adjective": "colorful",
  "noun": "bird"
};

function interpolate(string, values) {
  return string.replace(/\{\{\s*(.*?)\s*\}\}/g, function (_, key) {
    return interpolate(values[key] || "", values);
  });
}

function fillInTheBlanks(template, key) {
  return interpolate(template[key], template);
}

// output (just for the stack snippet)
document.write( fillInTheBlanks(template, 'sentence') );

In this case, fillInTheBlanks is just there to kick things off. The actual work is done in interpolate. That function looks for instances of {{...}} in its input string, and then replaces it with the value for whatever key is between in the braces (ignoring spaces and such). And it calls itself on the resulting string, in order to achieve the recursive replacement behavior.

The name interpolate comes from "string interpolation", the general name for "filling in the blanks" in a template string.

The regular expression can be explained like so:

\{\{          // look for two opening braces "{{"
\s*           // followed by zero or more whitespace characters
(             // start a capture group
  .*?         // match anything that's matched by the following (ungreedy matching)
)             // end capture group
\s*           // match zero or more whitespace characters
\}\}          // match the close braces "}}"

You could skip the \s* parts, and instead match everything between {{ & }} and use trim to remove any leading/trailing whitespace afterward. However, trim isn't always available while the regex should work anywhere.

Anyway, that should match "{{foo}}", "{{ foo}}", "{{foo }}", etc.. The capture group (group 1) will only match foo within that. The g flag after the regex means "global" - i.e. "replace all", not just the first match.

So when used with replace, the function passed in will receive the entire matched part of the string (e.g. "{{ foo }}") as its 1st argument. We don't care about the entire thing though, so the argument is just named _ in the above to denote we're not using it for anything (not to be confused with underscore/lodash js).

As its 2nd argument, the function receives the contents of the 1st capture group (e.g. "foo"), which is what we're interested in. (Had there been more capture groups, they would've been passed in as the 3rd, 4th, etc. arguments.) So we take that capture group's contents, and treat it as a key/property name, and look it up in the values object. Should the key not actually exist, values[key] will evaluate to undefined, so we handle that by defaulting to an empty string.

In other words, values[key] || "" reads as "get the value for key, or, if it's missing, use an empty string".

And that is then passed through interpolate again, so the string is built recursively.

The danger with this is that the templating can itself be recursive. If, for instance, you call:

var values = {
  foo: "{{ bar }}",
  bar: "{{ foo }}"
};

interpolate("{{ foo }}", values);

it'll endlessly bounce back and forth, replace one with the other. In practical terms, JavaScript will of course just give up (i.e. raise an exception and stop dead) because the stack level - interpolate calling interpolate ad infinitum - will get too deep.

Your solution would enter an infinite loop instead, as it uses a while loop.

How to deal with such pathological input is outside the scope of this review, but would've been a good question to ask the interviewers. (Personally, I'd just let JS give up; the best you can do is write a bunch more code... and also give up, since there's no good solution. You can be more graceful about it and provide an error message, but that's about it.)


On a side note, the task seems a little weird. Why have fillInTheBlanks take a "raw" key, when the task is supposedly about templating? It'd make more sense to skip straight to interpolate("{{ sentence }}", template), as that's the core of the task. It's also why I chose to expose interpolate in the same scope as fillInTheBlanks - it could just as well be nested inside fillInTheBlanks, but it's far more useful when it's accessible.

I'd also wonder why the object holding the keys and values is called template. That makes no sense and is in fact backwards: The template is the string, and the object holds the values you use to fill in that template. Hence why I called it values in my function.

\$\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.