Repository: neherlab/covid19_scenarios
File: src/components/Main/Results/DeterministicLinePlot.tsx (TypeScript)

Code before:

  // this currently relies on there being data for every day. This should be
  // the case given how the data are parsed, but would be good to put in a check
  const newCases =
    caseDelta === 0
      ? (cc: EmpiricalData, i: number) => {
          if (i >= caseBaseStep && cc[i].cases !== null && cc[i - caseBaseStep].cases !== null) {
            return verifyPositive((cc[i].cases as number) - (cc[i - caseBaseStep].cases as number))
          }
          return undefined
        }
      : (cc: EmpiricalData, i: number) => {
          if (
            i >= caseBaseStep + 1 &&
            cc[i].cases !== null &&
            cc[i - caseBaseStep + 1].cases !== null &&
            cc[i - caseBaseStep].cases !== null
          ) {
            return verifyPositive(
              (cc[i].cases as number) -
                ((1 - caseDelta) * (cc[i - caseBaseStep].cases as number) +
                  caseDelta * (cc[i - caseBaseStep + 1].cases as number)),
            )
          }
          return undefined
        }

  const countObservations = {
    cases: nonEmptyCaseCounts?.filter((d) => d.cases).length ?? 0,
    ICU: nonEmptyCaseCounts?.filter((d) => d.icu).length ?? 0,
    observedDeaths: nonEmptyCaseCounts?.filter((d) => d.deaths).length ?? 0,
    newCases: nonEmptyCaseCounts?.filter((_, i) => newCases(nonEmptyCaseCounts, i)).length ?? 0,
    hospitalized: nonEmptyCaseCounts?.filter((d) => d.hospitalized).length ?? 0,
  }

  const observations =
    nonEmptyCaseCounts?.map((d, i) => ({
      time: new Date(d.time).getTime(),
      cases: enabledPlots.includes(DATA_POINTS.ObservedCases) ? d.cases || undefined : undefined,
      observedDeaths: enabledPlots.includes(DATA_POINTS.ObservedDeaths) ? d.deaths || undefined : undefined,
      currentHospitalized: enabledPlots.includes(DATA_POINTS.ObservedHospitalized)
        ? d.hospitalized || undefined
        : undefined,
      ICU: enabledPlots.includes(DATA_POINTS.ObservedICU) ? d.icu || undefined : undefined,
      newCases: enabledPlots.includes(DATA_POINTS.ObservedNewCases) ? newCases(nonEmptyCaseCounts, i) : undefined,
      hospitalBeds: nHospitalBeds,
      ICUbeds: nICUBeds,
    })) ?? []

  const plotData = [
    ...data.deterministic.trajectory.map((x) => ({

Review comment (refactor):
I don't know how to read this. Can we please agree on limiting ternary operators to one-liners only?

As there are 4 booleans combined in an if statement, it makes me think that this particular check does something very specific. But I cannot immediately see what. Can we refactor it into a named function, so that the name would give me a hint?

Please refactor all duplicated statements like `i - caseBaseStep` and `cc[i - caseBaseStep]` into well-named variables.

Why type-casting (`as number`) is needed? I am trying to introduce type checks into the codebase and casts go against that. After cast is written it is extremely hard to find it. I'd prefer you leave type errors that I can see and fix later, or even better that you fix them right away.

Do we really need to produce 2 arrow functions conditionally? Can this condition be moved inside the function?
Can then this function be a normal named `function` in the outer scope, so that the component contains less fluff?


Code after:
const verifyPositive = (x: number) => (x > 0 ? x : undefined)

// FIXME: this component has become too large
// eslint-disable-next-line sonarjs/cognitive-complexity
export function DeterministicLinePlot({
  data,
  userResult,
  params,
  mitigation,
  logScale,
  showHumanized,
  caseCounts,
  forcedWidth,
  forcedHeight,
}: LinePlotProps) {
  const { t } = useTranslation()
  const chartRef = React.useRef(null)
  const [enabledPlots, setEnabledPlots] = useState(Object.values(DATA_POINTS))

  // RULE OF HOOKS #1: hooks go before anything else. Hooks ^, ahything else v.
  // href: https://reactjs.org/docs/hooks-rules.html

  const formatNumber = numberFormatter(!!showHumanized, false)
  const formatNumberRounded = numberFormatter(!!showHumanized, true)

  // FIXME: is `data.stochasticTrajectories.length > 0` correct here?
  if (!data || data.stochastic.length > 0) {
    return null
  }

  const { mitigationIntervals } = mitigation

  const nHospitalBeds = verifyPositive(data.params.hospitalBeds)
  const nICUBeds = verifyPositive(data.params.ICUBeds)

  const nonEmptyCaseCounts = caseCounts?.filter((d) => d.cases || d.deaths || d.icu || d.hospitalized)

  const [newEmpiricalCases, caseTimeWindow] = computeNewEmpiricalCases(
    params.epidemiological.infectiousPeriod,
    verifyPositive,
    nonEmptyCaseCounts,
  )

  const countObservations = {
    cases: nonEmptyCaseCounts?.filter((d) => d.cases).length ?? 0,
    ICU: nonEmptyCaseCounts?.filter((d) => d.icu).length ?? 0,
    observedDeaths: nonEmptyCaseCounts?.filter((d) => d.deaths).length ?? 0,
    newCases: nonEmptyCaseCounts?.filter((_0, i) => newEmpiricalCases[i]).length ?? 0,
    hospitalized: nonEmptyCaseCounts?.filter((d) => d.hospitalized).length ?? 0,
  }