I'm trying to combine these 2 functions in an elegant way. I already reduce the complexity by using switch statement and const variable for repeating value
This functions subtracts day values by given day, week or 2week
const handleLeft = (
active: any,
setActiveDate: any,
setParagonActiveDate: any
) => {
const time = (prevDay: any, length: number) =>
moment(prevDay)
.subtract(length, 'd')
.format('YYYY/MM/DD');
switch (active) {
case 'day':
setActiveDate((prevDay: any) => time(prevDay, 1));
setParagonActiveDate &&
setParagonActiveDate((prevDay: any) => time(prevDay, 1));
break;
case 'week':
setActiveDate((prevDay: any) => time(prevDay, 6));
setParagonActiveDate &&
setParagonActiveDate((prevDay: any) => time(prevDay, 6));
break;
case '2weeks':
setActiveDate((prevDay: any) => time(prevDay, 13));
setParagonActiveDate &&
setParagonActiveDate((prevDay: any) => time(prevDay, 13));
}
};
This functions adds day values by given day, week or 2week
const handleRight = (
active: any,
setActiveDate: any,
setParagonActiveDate?: any
) => {
const time = (prevDay: any, length: number) =>
moment(prevDay)
.add(length, 'd')
.format('YYYY/MM/DD');
switch (active) {
case 'day':
setActiveDate((prevDay: any) => time(prevDay, 1));
setParagonActiveDate &&
setParagonActiveDate((prevDay: any) => time(prevDay, 1));
break;
case 'week':
setActiveDate((prevDay: any) => time(prevDay, 6));
setParagonActiveDate &&
setParagonActiveDate((prevDay: any) => time(prevDay, 6));
break;
case '2weeks':
setActiveDate((prevDay: any) => time(prevDay, 13));
setParagonActiveDate &&
setParagonActiveDate((prevDay: any) => time(prevDay, 13));
}
};
The only difference is whether you do .subtract
or .add
after moment
, which you can do in a single function using the conditional operator inside bracket notation and a fourth argument:
const handleLeftOrRight = (
active: any,
setActiveDate: any,
setParagonActiveDate: any,
add: boolean
) => {
const time = (prevDay: any, length: number) =>
moment(prevDay)
[add ? 'add' : 'subtract'](length, 'd')
.format('YYYY/MM/DD');
// rest of your code
};
Then use, eg
handleLeftOrRight(active, setActiveDate, setParagonActiveDate, true)
instead of
handleRight(active, setActiveDate, setParagonActiveDate)
and the same for passing false
as a fourth argument instead of handleLeft
.
But since you're using TypeScript, I'd also highly recommend typing things properly, to actually take advantage of TypeScript's type system - in TypeScript, it's almost always a good idea to avoid any
, since it's not type-safe - it can be assigned to anything and used anywhere, opening up the possibility of type-related runtime errors or bugs. For example, active
can be either 'day'
, 'week'
, or '2weeks'
, so you can do:
const handleLeftOrRight = (
active: 'day' | 'week' | '2weeks',
instead of using any
.
Another way to improve the code and make things a lot DRYer would be to have an object mapping the active
value to the second argument passed to time
.
const daysByActive = {
day: 1,
week: 6,
'2weeks': 13
}
const handleLeftOrRight = (
active: 'day' | 'week' | '2weeks',
setActiveDate: any,
setParagonActiveDate: any, // this might be improved by changing to boolean?
add: boolean
) => {
const time = (prevDay: any, length: number) =>
moment(prevDay)
[add ? 'add' : 'subtract'](length, 'd')
.format('YYYY/MM/DD');
const days = daysByActive[active];
setActiveDate((prevDay: any) => time(prevDay, days));
if (setParagonActiveDate)
setParagonActiveDate((prevDay: any) => time(prevDay, days));
};