CODEWITHSUNDEEP
javascriptjquery
peduarte
peduarte

Reputation: 1677

Javascript function tidy up

Below you'll see a javascript/jQuery function I'm currently using:

            var forward = new RegExp('forward'),
                backward = new RegExp('backward'),              
                left = new RegExp('left'),      
                right = new RegExp('right');                

            if ( tweet.message.match(forward) ) {
                console.log('forward');
                body.addClass('forward');
                bodyRemoveClass();                          
            }

            if ( tweet.message.match(backward) ) {
                console.log('backward');
                body.addClass('backward');
                bodyRemoveClass();                          
            }

            if ( tweet.message.match(left) ) {
                console.log('left');
                body.addClass('left');
                bodyRemoveClass();                          
            }

            if ( tweet.message.match(right) ) {
                console.log('right');
                body.addClass('right');
                bodyRemoveClass();                          
            }

Everything works just fine, but I'm not 100% happy with the way it's written. Basically what that does is, check if a given keyword (forward, backward, left or right)
are in a tweet (tweet.message)

I would like to know if there is a simple/cleaner way to achieve this.

Sorry but there is no online example...

Thank you

Upvotes: 1

Views: 255

Answers (5)

jfriend00
jfriend00

Reputation: 707386

You can use a table-drive approach like this:

var tags = ["forward", "backward", "left", "right"];
for (var i = 0; i < tags.length; i++) {
    if (tweet.message.indexOf(tags[i]) != -1) {
        body.addClass(tags[i]);
        bodyRemoveClass();
    }
}

Keep in mind that any time you're repeating the same logic and the same strings over and over again, you should find a way to either drive your logic by iterating through a table, break the code out into a common function or something that prevents you from repeating code over and over again. The popular lingo in software development these days for this is DRY (don't repeat yourself).

There was also no reason for test each individual match with a regular expression. While they can be very handy when needed, .indexOf() is a lot faster when that's all that is needed.

Upvotes: 4

Aaron
Aaron

Reputation: 5227

var directions = new Array('forward', 'backward', 'left', 'right');
$(directions).each(function()
{
    var regX = new RegExp(this);
    if (tweet.message.match(regX))
    {
        console.log(this);
        body.addClass(this);
        bodyRemoveClass();
    }
});

Upvotes: 1

Wayne
Wayne

Reputation: 60414

This can be drastically shortened by capturing the matched phrase:

var match = tweet.message.match(/forward|backward|left|right/g);
for (var i = 0; match && i < match.length; i++) {
    console.log(match[i]);
    body.addClass(match[i]);
    bodyRemoveClass();
}

The only thing different in each of those if blocks is the phrase to be matched (which is the same as the name of the class to be added). Repetition like that can (and should) almost always be avoided.

Upvotes: 1

JayC
JayC

Reputation: 7141

function TweetMatch(classToAdd){
    if ( tweet.message.match(new RegExp(classToAdd))) {
        console.log(classToAdd);
        body.addClass(classToAdd);
        bodyRemoveClass();                          
    }
}
TweetMatch('forward');
TweetMatch('backward');
TweetMatch('left');
TweetMatch('right');

etc. (I see fellow commenters have gone ahead and made arrays to iterate over... depends upon how many directions you want :-D)

Upvotes: 3

Michael Berkowski
Michael Berkowski

Reputation: 270637

There's no need to use match() with Regexp here. You can do simple string matching with indexOf(). You can then avoid declaring all the Regexp at the beginning.

        if ( tweet.message.indexOf("forward") > -1) {
            console.log('forward');
            body.addClass('forward');
            bodyRemoveClass();                          
        }

        if ( tweet.message.indexOf("backward") > -1) {
            console.log('backward');
            body.addClass('backward');
            bodyRemoveClass();                          
        }

        if ( tweet.message.indexOf("left") > -1) {
            console.log('left');
            body.addClass('left');
            bodyRemoveClass();                          
        }

        if ( tweet.message.indexOf("right") > -1) {
            console.log('right');
            body.addClass('right');
            bodyRemoveClass();                          
        }

However, this is much more neatly accomplished with an array of classes:

// Store your 4 classes in an array
var classes = ["forward","backward","left","right"];
for (var i = 0; i<classes.length; i++) {

  // Check each class appearing in the message:
  if ( tweet.message.indexOf(classes[i]) > -1) {
     console.log(classes[i]);
     body.addClass(classes[i]);
     bodyRemoveClass();                          
  }
}

Upvotes: 4

Related Questions