Search code examples
javascriptreactjsfunctional-programmingfuncreusability

Best way to combine these 2 javascript functions


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));
  }
};

Solution

  • 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));
    };