8
\$\begingroup\$

I was doing a JavaScript challenge to test myself and I succeeded in finding a solution but the code is very ugly. I was hoping to get some opinions on what a better solution would look like.

The problem is this. We need to write, in Javascript, a function with signature

f(input)

that when called in the following ways give the following outputs

f("a") => "fa"
f()("a") => "foa"
f()()("a") => "fooa"

and so on, with the number of 'o''s being equal to the number of empty parentheses.

Here is my solution

function f(input) {
  function inner(otherInput){ 
    if(typeof otherInput === 'undefined') 
        if(typeof input === 'undefined')
            return f({acc:'foo'});
        else
            return f({acc:input.acc + 'o'});

    if(typeof input !== 'undefined') 
        return input.acc + otherInput;
    else 
        return 'fo' + otherInput;
  };
  if(typeof input === 'string')
    return "f" + input;
  return inner;
}

How can this be improved?

\$\endgroup\$
  • 1
    \$\begingroup\$ Introducing the accumulator as a default argument makes the function more flexible and shorter at the same time: f = (str, acc = "f") => str ? acc + str : str => f(str, acc + "o"); \$\endgroup\$ – le_m Nov 24 '17 at 18:25
  • \$\begingroup\$ @le_m I did the same but I used an integer as accumulator, so I had to "o".repeat(acc). \$\endgroup\$ – Gabriel Nov 24 '17 at 18:49
  • \$\begingroup\$ @Gabriel Good idea; you could combine that with hello_world's solution below. \$\endgroup\$ – le_m Nov 24 '17 at 19:29
12
\$\begingroup\$

Review on code.

The term "ugly" is subjective. For some ugly is bad practice or anti-patterns, and there are others that will argue that current standards of good practice are "ugly" due to inefficiencies.

I will take ugly in this case to mean inelegant in the mathematical sense of the term. Thus below is a review in terms of improving code for simplicity and efficiency.

Simplify.

There is no reason to use statement clause for undefined in the form typeof blah === "undefined". Assuming that no input will equate to false apart from no arguments then if(! input) { is simpler.

However, there may be an input like f()(false), where the expected output is "fofalse", which will require a more robust test for undefined. So the safe option without knowledge of possible input is if(input === undefined){ which is still simplier (and more efficient).

For each call of the function f there are only two possible return values, the string that terminates the chain of calls or the function that is used to expand the return string.

Your inner function has 4 return possibilities, which is an obvious unneeded complication.

Simplicity also means not to repeat code. The outer function and inner function have the same functionality. Thus the outer function only needs to begin a process of building a string. This can be done via a closure, so all the outer function need do is define a communicating variable and a function to hold the closure. It does not need to do any form of logic.

Efficiency

Simplicity and efficiency go hand in hand.

The inner function should only return a string or a function to do the same.

The communication between the function calls is where you have added too much overhead with the creation of objects to communicate to the outer function.

You can use a closure variable to communicate information between calls and reduce the call stack depth by not needing to call the outer function. The closure is maintained by returning the inner function in the case of an undefined input.

The result.

The result is hopefully less ugly in your eyes. For me it is far simpler and nicely elegant.

function f(str) {
    var content = "f"; 
    return (function inner(str){      
        if (str !== undefined) { return content + str }
        content += "o";
        return inner;
    })(str);          
}

console.log(f("a"));
console.log(f()("a"));
console.log(f()()("a"));

Alternative.

I did toy with the following version but for me it is not as elegant due to the forced flow unique to Javascript that will put those from other languages out of joint. Though it is simpler in terms of token count but not in complexity.

function f(str) {
    var content = "f"; 
    return (function inner(str){      
        return str ? content + str : ( content += "o", inner );
    })(str);          
}

console.log(f("a"));
console.log(f()("a"));
console.log(f()()("a"));

\$\endgroup\$
  • \$\begingroup\$ Can you please explain the last curry call f()()("a"). Are any curry function after f() just a call to inner? \$\endgroup\$ – Pavlo Nov 24 '17 at 15:01
  • 1
    \$\begingroup\$ @Pavio All but the first call are to inner. The first call sets up the closure over the var content and then calls inner. \$\endgroup\$ – Blindman67 Nov 24 '17 at 15:14
  • \$\begingroup\$ Oh i get it now, it basically like saying var f = f() then calling f(), which appends content, then calling f("a") which returns the content, then i can call f() again to append more \$\endgroup\$ – Pavlo Nov 24 '17 at 15:27
  • \$\begingroup\$ If you had aimed for elegancy in the mathematical sense I would have expected a functional solution that did not use mutation. I would definitely have expected fo = f(); console.log(fo("g"), fo("x")) to log fog and fox. \$\endgroup\$ – Bergi Nov 26 '17 at 15:19
  • \$\begingroup\$ @Bergi What?? there is no mutation, f remains as is, and the result of fo = f(); console.log(fo("g"), fo("x")) is "fog , fox" maybe you should run it and see.. Also functional programming is an irrelevant joke that has no place elegant JS that will be all but forgotten in a few years. \$\endgroup\$ – Blindman67 Nov 26 '17 at 22:11
7
\$\begingroup\$

Don't do that trickery with the accumulator that might be an object, just always have an accumulator value. And don't treat the first invocation as a special case:

function makeAppender(acc) {
    return function(input) {
        if (typeof input == "string")
            return acc + input;
        else
            return makeAppender(acc + "o");
    };
}
var f = makeAppender("f");
\$\endgroup\$
5
\$\begingroup\$

This might be of help?

const f = (a, acc = '') => a ? `f${acc}${a}` : (a) => f(a, `${acc}o`);

console.log(f('a'));
console.log(f()('a'));
console.log(f()()('a'));

Or with the proposed partial application (stage 1) syntax:

const f = (a, acc = '') => a ? `f${acc}${a}` : f(?, `${acc}o`);
\$\endgroup\$
4
\$\begingroup\$

You could have some fun with Function#bind.

function  f() {
    return arguments.length
        ? (this.value || 'f') + arguments[0]
        : f.bind({ value: (this.value || 'f') + 'o' });
}

console.log(f('a'));
console.log(f()('a'));
console.log(f()()('a'));
console.log(f()()()('a'));

\$\endgroup\$
4
\$\begingroup\$

@Blindman67's answer is great, with the small caveat that interleaving calls to the returned function combines results because they're closed over the same content. That may or may not be intended, depending on how you're counting () calls.

const f2 = f();
const complete1 = f2();
const complete2 = f2();

console.log(complete1("a")); // foooa, not fooa
console.log(complete2("a")); // foooa, not fooa

If that's not the intended behaviour and they should be counted separately, your original work was on the right track, but it's got some repetition and special cases. You can avoid that by moving everything into the inner function and calling it immediately:

function f(suffix) {
    return (function step(prefix, suffix) {
        if (suffix !== undefined) {
            return prefix + suffix;
        }
        else {
            return (suffix) => step(prefix + "o", suffix);
        }
    })("f", suffix);
}

console.log(f("a")) // fa
console.log(f()("a")) // foa
console.log(f()()("a")) // fooa

const f2 = f();
const complete1 = f2();
const complete2 = f2();

console.log(complete1("a")); // fooa
console.log(complete2("a")); // fooa
\$\endgroup\$
2
\$\begingroup\$

You don't need to store the output as you iterate, just store how many times the function has been called with empty parameters and then generate the final output when you return from the last call.

Here's a solution using ES6 features.

let f = ((i = 0) => v => v !== undefined && `f${Array(i).fill('o').join``}${v}` || (i++, f))();

console.log(f()()()()()('a'));
\$\endgroup\$
0
\$\begingroup\$

Let there be lambdas and let there be partial applications

var t = os => c => c ? "f" + os + c : t(os+"o"),
    f = t("");

console.log(f("a"));
console.log(f()("a"));
console.log(f()()("a"));
console.log(f()()()("a"));

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