CODEWITHSUNDEEP
javascriptloopsrefactoring
Billy Moon
Billy Moon

Reputation: 58521

Remove repeating code

How would I write this code without all the duplication

    // I have a loop which decrements m each time
    // set m to the starting point
    m = mid
    // set f to a calculated array value
    f = dict[l].substr( l * --m, l )
    while (f.substr(0,x) === word && (!limit || matches.length < limit)){
        matches.push(f);
        // same as what was defined outside the while loop
        // which seems to me like unnecessary repetition
        f = dict[l].substr( l * --m, l )
    }
    // then I repeat it all, but incrementing m
    // reset m to the starting point
    m = mid
    f = dict[l].substr( l * m++, l )
    while (f.substr(0,x) === word && (!limit || matches.length < limit)){
        matches.push(f);
        f = dict[l].substr( l * m++, l )
    }

There are two parts...

  1. each block contains a repeating f = ... part
  2. the blocks are repeated, only changing the increment/decrement of m

EDIT: What the code does...

mid represents an arbitrary entry point into an alphabetically sorted list of fixed length words with no separators. I am aiming to list all the words that match a set prefix, so must find all the words backwards of the arbitrary mid point (as entered by binary search method) and also forwards.

Edit: Further details...

dictionary looks like this:

dict = [
  "", // offset other values to equal word length
  "ai",    // all length 1
  "inatonuptogo",    // all length 2
  "catcrydogendgodhamhathit",    // all length 3
  "foodhackhallhandhardhatehatshavehellridewood" // all length 4
]

l is word length of searched words, so dict[l] is one row of words from the dictionary, of length l.

I am modifying John Resig's binary search method so that it will match a prefix rather than a whole word, and return a set of results, rather than a truthy value. I am also putting a limit in there, as I will be using this for an autocomplete function which only requires a few returned values, not all matches.

Upvotes: 0

Views: 103

Answers (3)

moopet
moopet

Reputation: 6175

Put it in a function and pass in a delta. Since m++ is adding +1 to m and m-- is adding -1 to m, you can call for example:

 function foo(mid, dict, delta) {
 m = mid
 //   f = dict[l].substr( l * m += delta, l )
    f = dict[l].substr( l * m, l );
    m += delta;
    while (f.substr(0,x) === word && (!limit || matches.length < limit)){
        matches.push(f);
        // f = dict[l].substr( l * m += delta, l )
        f = dict[l].substr( l * m, l );
        m += delta;
    }
    return matches;
}

matches = foo(mid, dict, -1);
matches = foo(mid, dict, +1);

I'm only pulling out matches as a return value because I'm not sure what your code is trying to do, but it should show you the idea.

Upvotes: 2

lcardosobr
lcardosobr

Reputation: 101

@Billy Moon, can you validate this code below?

function foo(flag, m) { return dict[l].substr( l * (Boolean(flag)) ? --m : m++), l ); }
var flag = 1;
do {
    m = mid
    f = foo(flag, m)
    while (f.substr(0,x) === word && (!limit || matches.length < limit)){
        matches.push(f);
        f = foo(flag, m)
    }
    flag--;
} while(flag > -1)

I hope may help you.

Upvotes: 0

pb2q
pb2q

Reputation: 59607

You might pull the loops into a function, since aside from the increment/decrement, they are identical:

function helperFunction(m, f, l, mid, dict, matches, limit, increment)
{
    m = mid;

    if (increment)
        f = dict[l].substr(l * m++, l);
    else f = dict[l].substr(l * --m, l);

    while (f.substr(0, x) === word && (!limit || matches.length < limit))
    {
        matches.push(f);
        f = dict[l].substr(l * m, l);

        if (increment)
            f = dict[l].substr(l * m++, l);
        else f = dict[l].substr(l * --m, l);
    }
}

I've included all the variables in the argument list since their scope is unclear.

Upvotes: 2

Related Questions