Method Names
The most method names do not describe what the method really does and what the method name implies.
getButtonData
let me imply that it returns information about one buttongetButtonDataByPage
the same likegetButtonData
add
implies that something gets added to a collection
In generell method names that start with get
are known as getter and that they return values of an object that has no or less computation. The methods you provide starts with get
but relay heavily on computation and do not return just a value.
Names that would better fit are: buildInitialButtonList
, buildNavigatedButtonList
and createButton
.
Hard to Read
if (pageSize > 10) { if (i < 6) { buttons.push(add(i, i == 1 ? true : false, true, false)); } else if (pageSize >= 6 && i == 6) { buttons.push(add(i, i == 1 ? true : false, pageSize > 6 ? false : true, >pageSize > 6 ? true : false)); } else if (i == pageSize) { buttons.push(add(i, false, true, false)); } }
The code smells boolean flags and magic numbers make your code hard to read an to maintain.
Imagine you want to show only 3 instead 6 buttons: Alone in this small snipped you have to touch the code 5 times and in the whole code you provide 12 times.
Many Conditions
Beside the code smells you have many conditions. The 3 lines below contain 4 conditions:
else if (pageSize >= 6 && i == 6) { buttons.push(add(i, i == 1 ? true : false, pageSize > 6 ? false : true, pageSize > 6 ? true : false)); }
Simplify Conditions
Some conditions follow the semantic of condition ? true : false
which is the same as condition
. For instance:
buttons.push(add(i, i == 1 ? true : false, pageSize > 6 ? false : true, pageSize > 6 ? true : false))
Is the same as
buttons.push(add(i, i == 1, pageSize > 6, pageSize > 6))
Useless Conditions
Let's analyze getButtonData
and the same will be apply to getButtonDataByPage
.
else if (... && i == 6) { buttons.push(add(i, i == 1 ? true : false, ..., ...)); }
We first check if (i == 6)
and if this is true we check if i == 1
which will always be false:
else if (pageSize >= 6 && i == 6) {
buttons.push(add(i, false, ..., ...));
}
if (pageSize > 10) { if (...) {/* ... */} else if (pageSize >= 6 && i == 6) { buttons.push(add(..., ..., pageSize <= 6, pageSize > 6)); } }
First we make sure that pageSize > 10
and than we check if pageSize >= 6
, pageSize <= 6
and pageSize > 6
. Since we already know that pageSize
is greater than 10 we can simplify to:
if (pageSize > 10) {
if (...) {/* ... */}
else if (i == 6) {
buttons.push(add(..., ..., true, true));
}
}
After the simplification:
for (let i = 1; i <= pageSize; i++) {
if (pageSize > 10) {
if (i < 6) {
buttons.push(add(i, i == 1, true, false));
}
else if (i == 6) {
buttons.push(add(i, false, true, true));
}
else if (i == pageSize) {
buttons.push(add(i, false, true, false));
}
}
else {
buttons.push(add(i, i == 1, true, false));
}
}
where i == 1
can have its own branch:
if (i === 1) {
buttons.push(add(i, true, true, false))
} else if (pageSize > 10) {
if (i < 6) {
buttons.push(add(i, false, true, false));
} else if (i === 6) {
buttons.push(add(i, false, true, true));
} else if (i === pageSize) {
buttons.push(add(i, false, true, false));
}
} else {
buttons.push(add(i, false, true, false));
}
Since only the first 6 button gets rendered we do not need to check for pageSize > 10
:
if (i === 1) {
buttons.push(add(i, true, true, false))
} else if (i < 6) {
buttons.push(add(i, false, true, false));
} else if (i === 6) {
buttons.push(add(i, false, true, true));
} else if (i === pageSize) {
buttons.push(add(i, false, true, false));
}
and since the branch body of i < 6
and i === pageSize
are the same:
if (i === 1) {
buttons.push(add(i, true, true, false))
} else if (i < 6 || i === pageSize) {
buttons.push(add(i, false, true, false));
} else if (i === 6) {
buttons.push(add(i, false, true, true));
}
Builder Pattern
To go away from all the boolean flags you could create a Builder and refactor the simplified version from above to something like:
const firstButton = new ButtonBuilder().withValue(1).disabled().visible().noPlaceholder();
const placeholder = new ButtonBuilder().enabled().visible().placeholder();
const button = new ButtonBuilder().enabled().visible().noPlaceholder();
for (let i = 1; i <= pageSize; i++) {
if (i === 1) {
buttons.push(firstButton);
} else if (i < 6 || i === pageSize) {
buttons.push(button.withValue(i).build());
} else if (i === 6) {
buttons.push(placeholder.withValue(i).build());
}
}
The Algorithm
In your example, you can page through 50 buttons, of which only 6 are required to render. For each button click you have to loop 50 times to render 6 buttons again.
But actually you know directly which buttons are required to render without to loop 50 times. There are 3 cases:
- on the first to 5. button :
1, 2, 3, 4, 5 ... 50
- on button between 6. to 50.:
1, ... 6, [7], 8, ..., 50
- on the last button :
1, ... 46, 47, 48, 49, [50]
some pseudo code:
function buildPagination(current) {
if (current < 6) {
const pagination = [
button(1),
button(2),
button(3),
button(4),
button(5),
placeholder(),
button(last),
];
pagination[current].disable();
return pagination;
}
if (current === last) {
return [
button(1),
placeholder(),
button(last - 4),
button(last - 3),
button(last - 2),
button(last - 1),
button(last).disable(),
];
}
return [
button(1),
placeholder(),
button(current - 1),
button(current).disable(),
button(current + 1),
placeholder(),
button(last),
];
}