CODEWITHSUNDEEP
javascriptreactjsalgorithmfunctionbig-o
Mateen Kazia
Mateen Kazia

Reputation: 349

Combining 2 complex Javascript functions

Hello I have 2 complex functions that gives values to a Graph component but with 2 different data source.

This is the first Function that handle for 1 graph

const timeLine = (data: any, day: string) => {
// subtract 1 hour from the dates here since the chart day columns begin at the 23hr mark
const chartStartDate = moment(activeDate, 'YYYY/MM/DD').subtract(1, 'hour');
const dayStart: any = moment(day, 'MMM D').subtract(1, 'hour');
let nextDay = moment(dayStart);
nextDay = nextDay.add(1, 'day');
const finalDayData: any[] = [];
(data || []).forEach((activity: any, index: number) => {
  const offSetY = index;
  const format = 'YYYY-MM-DD h:mm:ss';
  const startMoment: any = moment(activity['START_DATE'], format);
  const endMoment = moment(activity['END_DATE'], format);
  if (
    (startMoment.isSameOrAfter(dayStart) &&
      startMoment.isBefore(nextDay)) ||
    (startMoment.isBefore(chartStartDate) &&
      dayStart.isSame(chartStartDate, 'day'))
  ) {
    let numberOfUnits;
    let offSetX;
    if (startMoment.isBefore(chartStartDate)) {
      // This bar starts before the start date of the entire chart, so we need to reduce the units so it doesn't overflow to the left.
      numberOfUnits = endMoment.diff(chartStartDate, 'hours', true);
      offSetX = 0;
    } else {
      numberOfUnits = endMoment.diff(startMoment, 'hours', true);
      offSetX = startMoment.diff(dayStart, 'hours', true);
    }
    finalDayData.push({
      numberOfUnits,
      offSetX,
      offSetY,
      unitColor: getUnitColor(activity.UNIT, userPlant)
    });
  }
});
return finalDayData;
};

This is the second function that is very similar to the first one but handles for a different dataset.

const fireRiskTimeLine = (data: any, day: string) => {
const activityTypes: string[] = [];
// subtract 1 hour from the dates here since the chart day columns begin at the 23hr mark
const chartStartDate = moment(activeDate, 'YYYY/MM/DD').subtract(1, 'hour');
const dayStart: any = moment(day, 'MMM D').subtract(1, 'hour');
let nextDay = moment(dayStart);
nextDay = nextDay.add(1, 'day');
const finalDayData: any[] = [];
(data || []).forEach((activity: any) => {
  const foundIdx = activityTypes.findIndex(
    (type: string) => activity.DESCRIPTION === type
  );
  if (foundIdx === -1) {
    activityTypes.push(activity.DESCRIPTION);
  }
  const offSetY = foundIdx === -1 ? activityTypes.length - 1 : foundIdx;
  const format = 'YYYY-MM-DD h:mm:ss';
  const startMoment: any = moment(activity['BEGIN DATE'], format);
  const endMoment = moment(activity['END DATE'], format);
  if (
    (startMoment.isSameOrAfter(dayStart) &&
      startMoment.isBefore(nextDay)) ||
    (startMoment.isBefore(chartStartDate) &&
      dayStart.isSame(chartStartDate, 'day'))
  ) {
    let numberOfUnits;
    let offSetX;
    if (startMoment.isBefore(chartStartDate)) {
      // This bar starts before the start date of the entire chart, so we need to reduce the units so it doesn't overflow to the left.
      numberOfUnits = endMoment.diff(chartStartDate, 'hours', true);
      offSetX = 0;
    } else {
      numberOfUnits = endMoment.diff(startMoment, 'hours', true);
      offSetX = startMoment.diff(dayStart, 'hours', true);
    }
    finalDayData.push({
      numberOfUnits,
      offSetX,
      offSetY,
      riskColor: activity['COLOR DESCRIPTION']
    });
  }
});
return finalDayData;
};

Would there be elegant way to combine these 2 functions or would it better be go ahead and leave as 2 separate functions. I tried to combine them but felt like I was over complicating it.

Upvotes: 1

Views: 49

Answers (2)

Flak
Flak

Reputation: 2670

I would do it different,

instead of combining I would split it with more functions eg.

main(data){
 let data = prepareData(data);
 let combineData = loopData(data);
 return combineData;
}

Break down

prepareData(data){

const chartStartDate = moment(activeDate, 'YYYY/MM/DD').subtract(1, 'hour');
const dayStart: any = moment(day, 'MMM D').subtract(1, 'hour');
let nextDay = moment(dayStart);
nextDay = nextDay.add(1, 'day');
const finalDayData: any[] = [];
}
loopData(data){
(data || []).forEach((activity: any) => {
  const foundIdx = activityTypes.findIndex(
    (type: string) => activity.DESCRIPTION === type
  );
  if (foundIdx === -1) {
    activityTypes.push(activity.DESCRIPTION);
  }
  const offSetY = foundIdx === -1 ? activityTypes.length - 1 : foundIdx;
  const format = 'YYYY-MM-DD h:mm:ss';
  const startMoment: any = moment(activity['BEGIN DATE'], format);
  const endMoment = moment(activity['END DATE'], format);
  if (
    (startMoment.isSameOrAfter(dayStart) &&
      startMoment.isBefore(nextDay)) ||
    (startMoment.isBefore(chartStartDate) &&
      dayStart.isSame(chartStartDate, 'day'))
  ) {
    let numberOfUnits;
    let offSetX;
    if (startMoment.isBefore(chartStartDate)) {
      // This bar starts before the start date of the entire chart, so we need to reduce the units so it doesn't overflow to the left.
      numberOfUnits = endMoment.diff(chartStartDate, 'hours', true);
      offSetX = 0;
    } else {
      numberOfUnits = endMoment.diff(startMoment, 'hours', true);
      offSetX = startMoment.diff(dayStart, 'hours', true);
    }
    finalDayData.push({
      numberOfUnits,
      offSetX,
      offSetY,
      riskColor: activity['COLOR DESCRIPTION']
    });
  }

Code is abstract but I would split it in small functions because if I need new data source tomorrow I would modify only prepareData() and not whole thing. my 2 cents :) }

Upvotes: 2

Edaz
Edaz

Reputation: 300

This is quite opinion-based, but if both functions do similar things, and you don't need to be able to call them separately, merging them into one seems like it would be a good idea as it would keep your code cleaner.

Upvotes: 2

Related Questions