Character and word count functionality

Question

I have this JS that has a lot of redundant functionality, and it works but looks unseemly.

I would love some assistance with making this more DRY (Don't Repeat Yourself):

// This provides character & word counter functionality

counter = function() {
    var body_value = $('#post_body').val();
    var title_value = $('#post_title').val();       

    if (body_value.length == 0) {
        $('#wordCountBody').html(0);
        $('#totalCharsBody').html(0);
        return;
    }

    if (title_value.length == 0) {
        $('#wordCountTitle').html(0);
        return;
    }

    var regex = /\s+/gi;
    var wordCountBody = body_value.trim().replace(regex, ' ').split(' ').length;
    var totalCharsBody = body_value.length;
    var wordCountTitle = title_value.trim().replace(regex, ' ').split(' ').length;

    $('#wordCountBody').html(wordCountBody);
    $('#totalCharsBody').html(totalCharsBody);
    $('#wordCountTitle').html(wordCountTitle);
};

$(document).ready(function() {
    $('#count').click(counter);
    $('#post_body').change(counter);
    $('#post_body').keydown(counter);
    $('#post_body').keypress(counter);
    $('#post_body').keyup(counter);
    $('#post_body').blur(counter);
    $('#post_body').focus(counter);
    $('#post_title').change(counter);
    $('#post_title').keydown(counter);
    $('#post_title').keypress(counter);
    $('#post_title').keyup(counter);
    $('#post_title').blur(counter);
    $('#post_title').focus(counter);        
});

Two pointers: Firstly, jQuery event handler assignments, like most jQuery selection methods, return this. Secondly, jQuery.on accepts an object parameter for assigning handlers to multiple events. — joews
– joews, Commented Sep 17, 2014 at 18:48
Why would you not want to count anything if either body or title are empty (but not necessarily the other)? — Bergi
– Bergi, Commented Sep 17, 2014 at 19:23
@Bergi - above counter. I updated the question to include that code. It was irrelevant to this issue, so I left it out for brevity. — marcamillion
– marcamillion, Commented Sep 17, 2014 at 19:52
If ready is irrelevant to the question, it's better to remove its usages not to add its declaration :-) — Bergi
– Bergi, Commented Sep 17, 2014 at 19:57

Alexis King · Accepted Answer · 2014-09-17 21:27:56Z

Your code looks mostly fine—if you're just performing this specific functionality in one place, some repetition is okay—but there's one thing that could definitely be simplified. Your ready function could be reduced to simply this:

$(document).ready(function () {
    $('#count').click(counter);
    $('#post_body, #post_title').on('change keydown keypress keyup blur focus', counter);
});

If, however, you really wanted to reduce all repetition, you could generalize the counter function until you ended up with a fairly reusable component.

Your counter function currently performs two actions (word counting and character counting) on two separate elements (the body and the title). To make the code as DRY as possible, you should employ the single responsibility principle to break up the function into composable bits.

function countUpdater(element, transformations) {
    return function () {
        var value = element.val();
        transformations.forEach(function (transformation) {
            transformation.display.html(value.length === 0 ? 0 : transformation.counter(value));
        });
    }
}

function wordCounter(value) {
    return value.trim().replace(/\s+/g, ' ').split(' ').length;
}

function characterCounter(value) {
    return value.length;
}

Your event handler code would then become this:

$(document).ready(function () {
    var changeEvents = 'change keydown keypress keyup blur focus';

    var bodyUpdater = countUpdater($('#post_body'), [
        { display: $('#wordCountBody'), counter: wordCounter },
        { display: $('#totalCharsBody'), counter: characterCounter },
    ]);
    var titleUpdater = countUpdater($('#post_title'), [
        { display: $('#wordCountTitle'), counter: wordCounter },
    ]);

    $('#count').click(function () {
        bodyUpdater();
        titleUpdater();
    });
    $('#post_body').on(changeEvents, bodyUpdater);
    $('#post_title').on(changeEvents, titleUpdater);
});

Is that better? Worse? Well, it's certainly less clear, but it's also much more reusable. If you're going to use this type of functionality many times, it might be worth it, but otherwise, I might prefer the simpler, more explicit version.

LOL...wow....thanks much for this. Yeh....I think it is definitely less clear. I like the top part of the post with the modification to the document.ready(function) though. — marcamillion
– marcamillion, Commented Sep 17, 2014 at 21:20
@marcamillion Yep, completely eradicating code repetition can often introduce layers of indirection or abstraction that you don't really need and can just clutter the code. In this case, I'm guessing the version you wrote it probably fine. — Alexis King
– Alexis King, Commented Sep 17, 2014 at 21:23
Very nice! I basically agree, I'd start with the asker's version and refactor only when I see I need that functionality elsewhere. I'd just like to add that another reason for refactoring could be to make the code easily testable. (Whether you want to test this stuff is arguable of course!) — nicolagi
– nicolagi, Commented Sep 18, 2014 at 7:08

Stack Exchange Network

Character and word count functionality

1 Answer 1

You must log in to answer this question.

Hot Network Questions

Character and word count functionality

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions