Page indicator - can it be smaller?

Question

I've just written this function for a test, but now I want to rewrite it for production.

I would like to know the best way to do this, or if this is "best practice" I think it's too long for the minimal stuff it does.

The whole idea is something like this: I have a one page with three sections floated left and a couple of subpages under each section.

X marks current page

-------------
| X | | |
-------------
| | | |
------------- 

Then I have an indicator that looks like my little map above - where X indicates which page is active etc...

 function pageIndicator() {
 var projectCount = $('.current > .project').size();
 var subcounter = $('.current > .page').size();
 console.log(settings.subcount);
 console.log(subcounter);
 $('.activated').removeClass('activated');
 $('.inactive').removeClass('inactive');
 // SECTION ONE
 if(settings.subcount == 1 && settings.count == 1) {
 $('.pagination ul li:nth-child(1)').addClass('activated');
 $('.pagination ul li:nth-child(3n+2), .pagination ul li:nth-child(3n+3)').addClass('inactive');
 $('.pagination ul li:nth-child(2), .pagination ul li:nth-child(3)').removeClass('inactive');
 }
 if(settings.subcount != 1 && settings.subcount < (projectCount + 2) && settings.count == 1) {
 $('.pagination ul li:nth-child(4)').addClass('activated');
 $('.pagination ul li:nth-child(3n+2), .pagination ul li:nth-child(3n+3)').addClass('inactive');
 $('.pagination ul li:nth-child(2), .pagination ul li:nth-child(3)').removeClass('inactive');
 }
 if(settings.subcount == (subcounter - 3) && settings.count == 1) {
 $('.pagination ul li:nth-child(7)').addClass('activated');
 $('.pagination ul li:nth-child(3n+2), .pagination ul li:nth-child(3n+3)').addClass('inactive');
 $('.pagination ul li:nth-child(2), .pagination ul li:nth-child(3)').removeClass('inactive');
 }
 if(settings.subcount == (subcounter - 2) && settings.count == 1) {
 $('.pagination ul li:nth-child(10)').addClass('activated');
 $('.pagination ul li:nth-child(3n+2), .pagination ul li:nth-child(3n+3)').addClass('inactive');
 $('.pagination ul li:nth-child(2), .pagination ul li:nth-child(3)').removeClass('inactive');
 }
 if(settings.subcount == (subcounter - 1) && settings.count == 1) {
 $('.pagination ul li:nth-child(13)').addClass('activated');
 $('.pagination ul li:nth-child(3n+2), .pagination ul li:nth-child(3n+3)').addClass('inactive');
 $('.pagination ul li:nth-child(2), .pagination ul li:nth-child(3)').removeClass('inactive');
 }
 if(settings.subcount == subcounter && settings.count == 1) {
 $('.pagination ul li:nth-child(16)').addClass('activated');
 $('.pagination ul li:nth-child(3n+2), .pagination ul li:nth-child(3n+3)').addClass('inactive');
 $('.pagination ul li:nth-child(2), .pagination ul li:nth-child(3)').removeClass('inactive');
 }
 // SECTION TWO
 if(settings.subcount == 1 && settings.count == 2) {
 $('.pagination ul li:nth-child(2)').addClass('activated');
 $('.pagination ul li:nth-child(3n+1), .pagination ul li:nth-child(3n+3)').addClass('inactive');
 $('.pagination ul li:nth-child(1), .pagination ul li:nth-child(3)').removeClass('inactive');
 }
 if(settings.subcount != 1 && settings.subcount < (projectCount + 2) && settings.count == 2) {
 $('.pagination ul li:nth-child(5)').addClass('activated');
 $('.pagination ul li:nth-child(3n+1), .pagination ul li:nth-child(3n+3)').addClass('inactive');
 $('.pagination ul li:nth-child(1), .pagination ul li:nth-child(3)').removeClass('inactive');
 }
 if(settings.subcount == (subcounter - 3) && settings.count == 2) {
 $('.pagination ul li:nth-child(8)').addClass('activated');
 $('.pagination ul li:nth-child(3n+1), .pagination ul li:nth-child(3n+3)').addClass('inactive');
 $('.pagination ul li:nth-child(1), .pagination ul li:nth-child(3)').removeClass('inactive');
 }
 if(settings.subcount == (subcounter - 2) && settings.count == 2) {
 $('.pagination ul li:nth-child(11)').addClass('activated');
 $('.pagination ul li:nth-child(3n+1), .pagination ul li:nth-child(3n+3)').addClass('inactive');
 $('.pagination ul li:nth-child(1), .pagination ul li:nth-child(3)').removeClass('inactive');
 }
 if(settings.subcount == (subcounter - 1) && settings.count == 2) {
 $('.pagination ul li:nth-child(14)').addClass('activated');
 $('.pagination ul li:nth-child(3n+1), .pagination ul li:nth-child(3n+3)').addClass('inactive');
 $('.pagination ul li:nth-child(1), .pagination ul li:nth-child(3)').removeClass('inactive');
 }
 if(settings.subcount == subcounter && settings.count == 2) {
 $('.pagination ul li:nth-child(17)').addClass('activated');
 $('.pagination ul li:nth-child(3n+1), .pagination ul li:nth-child(3n+3)').addClass('inactive');
 $('.pagination ul li:nth-child(1), .pagination ul li:nth-child(3)').removeClass('inactive');
 }
 // SECTION THREE
 if(settings.subcount == 1 && settings.count == 3) {
 $('.pagination ul li:nth-child(3)').addClass('activated');
 $('.pagination ul li:nth-child(3n+1), .pagination ul li:nth-child(3n+2)').addClass('inactive');
 $('.pagination ul li:nth-child(1), .pagination ul li:nth-child(2)').removeClass('inactive');
 }
 if(settings.subcount != 1 && settings.subcount < (projectCount + 2) && settings.count == 3) {
 $('.pagination ul li:nth-child(6)').addClass('activated');
 $('.pagination ul li:nth-child(3n+1), .pagination ul li:nth-child(3n+2)').addClass('inactive');
 $('.pagination ul li:nth-child(1), .pagination ul li:nth-child(2)').removeClass('inactive');
 }
 if(settings.subcount == (subcounter - 3) && settings.count == 3) {
 $('.pagination ul li:nth-child(9)').addClass('activated');
 $('.pagination ul li:nth-child(3n+1), .pagination ul li:nth-child(3n+2)').addClass('inactive');
 $('.pagination ul li:nth-child(1), .pagination ul li:nth-child(2)').removeClass('inactive');
 }
 if(settings.subcount == (subcounter - 2) && settings.count == 3) {
 $('.pagination ul li:nth-child(12)').addClass('activated');
 $('.pagination ul li:nth-child(3n+1), .pagination ul li:nth-child(3n+2)').addClass('inactive');
 $('.pagination ul li:nth-child(1), .pagination ul li:nth-child(2)').removeClass('inactive');
 }
 if(settings.subcount == (subcounter - 1) && settings.count == 3) {
 $('.pagination ul li:nth-child(15)').addClass('activated');
 $('.pagination ul li:nth-child(3n+1), .pagination ul li:nth-child(3n+2)').addClass('inactive');
 $('.pagination ul li:nth-child(1), .pagination ul li:nth-child(2)').removeClass('inactive');
 }
 if(settings.subcount == subcounter && settings.count == 3) {
 $('.pagination ul li:nth-child(18)').addClass('activated');
 $('.pagination ul li:nth-child(3n+1), .pagination ul li:nth-child(3n+2)').addClass('inactive');
 $('.pagination ul li:nth-child(1), .pagination ul li:nth-child(2)').removeClass('inactive');
 }
}

• Bill Barry: Could you maybe post an example on jsfiddle and link it here or post your html? I think having your page markup will help significantly.

Answer 1

Some suggestions: introduce a function for manipulating the classes, also you can check for the sections pretty easy and just once ... and actually, i'd stuff the bulks for the different sections in different methods. Determine the sections first, then pass the selected elements ...

function activate(el) {
 el.addClass('activated')
}
function toggle(el1, el2) {
 if (el1) el1.addClass('inactive');
 if (el2) el2.removeClass('inactive');
}
function pageIndicator() {
 $('.activated').removeClass('activated');
 $('.inactive').removeClass('inactive');
 var projectCount = $('.current > .project').size()
 , subcounter = $('.current > .page').size()
 , inSectionOne = settings.count == 1
 // SECTION ONE
 if (inSectionOne) {
 setUpSectionOne($('.pagination ul li:nth-child(3n+2), .pagination ul li:nth-child(3n+3)')
 , $('.pagination ul li:nth-child(2), .pagination ul li:nth-child(3)'))
 }
}
function setUpSectionOne(el1, el2) {
 var toggle = true;
 if (settings.subcount == 1) {
 activate($('.pagination ul li:nth-child(1)'));
 } else if (settings.subcount != 1 && settings.subcount < (projectCount + 2)) {
 activate($('.pagination ul li:nth-child(4)'));
 } else if (settings.subcount == (subcounter - 3)) {
 activate($('.pagination ul li:nth-child(7)'));
 } else if (settings.subcount == (subcounter - 2)) {
 activate($('.pagination ul li:nth-child(10)'));
 } else if (settings.subcount == (subcounter - 1)) {
 activate($('.pagination ul li:nth-child(13)'));
 } else if (settings.subcount == subcounter) {
 activate($('.pagination ul li:nth-child(16)'));
 } else {
 toogle = false;
 }
 if (toggle) {
 toggle(el1, el2);
 }
}

This is just for the first section, but it should be easy to apply it for the others as well.

• Jonas: I get a boolean error on the last if query
• Jonas: Solved it. Did trash the toggle function and added the classes directly instead.
• Florian Salihovic: I added a null in the toggle function, that might have been solved it as well.

Answer 2

Well, there's a few basic best practice tips I can give to start without really looking at the logic.

First, you can declare multiple variables in one statement, like so:

var projectCount = $('.current > .project').size(),
 subcounter = $('.current > .page').size();

Next, you access the same properties of an object multiple times, so you can avoid this and assign the value to a variable first.

var count = settings.count,
 subcount = settings.subcount;

Then

if(settings.subcount == (subcounter - 3) && settings.count == 1) {

becomes

if(subcount == (subcounter - 3) && count == 1) {

You can do the same thing with the class names that are used throughout:

var activated = 'activated',
 inactive = 'inactive';

This last step might seem pointless, but you can then run your script though a minifier such as http://dean.edwards.name/packer/ which can reduce your long variable names into something much shorter for production.

Looking at the logic it seems like you're repeating things an awful lot. For starters you should change your if statements to else...if as (I think) only one of these statements will be true, and if so there's no point in evaluating them all.

There's probably a more concise way to achieve the same logical result but I'll leave that to you!