How to refactor a react component that is handling too many conditions / use cases?

Viewed 1161

I'd like to apply SOLID principles with a react component that's handling too many 'use cases'. As an example let's say I have a component that's main responsibility is to render a table like so:

return (
   <Table dataSource="" name={tableName} />
)

This is a very simplistic representation of that table. But the main point of complexity is the prop dataSource. Inside this <Table> component, I actually have a lot of logic/if-else condition that's catering for the dataSource flag littered within the component because the dataSource can take in many different shapes / object structure. Some are very similar that they can be abstracted, while some are quite specific (and may even just differ by one key).

For example:

const tableName = `${
    dataSource === 'dataSourceA'
        ? dataSourceName
        : dataSourceName.name
}`;

This is just one example. Imagine some dataSource has its name property nested 3 levels deep. Then other dataSource will even have different names for the keys (although the resulting data that I need to render are actually the same). Not only that, depending on the dataSource, I may need to call a different endpoints to execute some functionalities (again, the function is doing the same purpose, just the endpoint can be different). Hence within the same component I'll have a function like:

const exportTable = () => {
    if(dataSource === 'dataSourceA') {
        // use endpoint A
    } else if (dataSource=== 'dataSourceB') {
        // use endpoint B
    } else {
         // use endpoint C
    }
}

What is the best way to refactor this kind of component and make it easier to maintain? Later on, we can have 10 types of dataSources and I can't be doing if-else conditions within the component to cater for their differences.

3 Answers

You can use hooks to abstract how the data is obtained:

const { rows, ...data } = useData(datasource)
return (
  <Table row={rows} {...data} />
);

In the useData hook:

function useData(dataSource) {
  if(dataSource === 'dataSourceA') {
      // use endpoint A
  } else if (dataSource=== 'dataSourceB') {
      // use endpoint B
  } else {
       // use endpoint C
  }

  // ...

  return data;
}

You will still have to deal with the conditionals, but they will be separated from the UI, which will make it easier to manage component lifecycle.

Secondly, you can make a service/API layer to abstract the data-fetching.

async function fetchFromEndpointA(args) {
  const response = await httpClient
      .get(`/endpointA/${args}`)

  return response.body;
}

The API layer would be consumed by the hooks:

// react-async useAsync is a lib that helps manage async state
import { useAsync } from 'react-async';

function useData(dataSource) {
  const a = useAsync({ promiseFn: fetchFromEndpointA, defer: true });
  const b = useAsync({ promiseFn: fetchFromEndpointB, defer: true });

  if(dataSource === 'dataSourceA') {
    const { run, error, isLoading data } = a;
    a.run();
    return { error, isLoading, data };
  }
  // ...
}

You could also abstract the datasource parsing+fetching from the hook. Without knowing the specifics of your datasource objects, I can only recommend a general strategy. It might look like:

async function obtainData(dataSource, parseableObject) {
  if(dataSource === 'dataSourceA') {
    return parseableObject['name'];
  } else if (dataSource === 'dataSourceB') {
    const name = parseableObject[0][0].Name; // whatever the path to name is
    const data = await callEndpointB(name);
    return data.result;
  } else {
    // ...
  }
}

Now any hook or component can call obtainData without needing to know the conditionals. The hook/component would only need to keep track of the async state.

For example, in a hook:

function useData(dataSource, parseableObject) {
  return useAsync({ promiseFn: () => obtainData(dataSource, parseableObject) });
}

Or, just call it in the component and forgo the custom hook altogether:

const { rows, ...data } = useAsync({ promiseFn: () => obtainData(dataSource, parseableObject) });
return (
  <Table row={rows} {...data} />
);

There are a lot of possibilities which you will probably need to explore before settling on a decision.

Lastly, some common advice on refactoring:

  • Before you abstract or refactor, first get all concrete high-level behavior correct with reliable, human-readable tests. Quick and dirty code is fine. Don't rely on testing implementation details or internal abstractions. Then if a clear pattern emerges later down the road, you can refactor using the existing tests to guide you.

  • Consider the costs. There's a programmer's proverb something along the lines of "having no abstraction is better than having the wrong abstraction." If you decide to reorganize your code into the separate layers, you need to be proactively aware of its affects on maintainability, etc. Check out WET Codebase. Rather than me summarize it here, it's best you watch/read it and glean your own insight.

I like bsapaka's answer. It's neat, and abstracts things well.

As an alternative, I have done what you've described in useEffect functions in the past.
Note: there is perhaps a case to be made for useMemo too, but I'll focus on useEffect.

import { parseTableData, TableData } from '../util/for/table/data';

interface TableProps {
  data: TableData; // Typing this would be super useful
  name: string;
}

const Table: React.FC<TableProps> = ({ name, data }) => {
  const [tableData, setTableData] = useState<TableData>(); // undefined on init

  useEffect(
    () => parseTableData(setTableData, data),
    [data, setTableData]
  );

  if (!tableData) {
    return <p>Parsing... please wait.</P>
  }

  // Here, you can use `tableData` knowing that it has been formatted properly

  return ...
}

Then, you can abstract your parser logic into a separate file:

interface TableData {
  ...
}

export const parseTableData = (
  setTableData: React.Dispatch<React.SetStateAction<TableData>>,
  data?: TableData,
) => {

  // Do whatever parsing / massaging you want to with the data here,
  //  and build the required response for the state variable.

  const response: TableData = {
    ...
  }

  setTableData(response);
}

In pure functional style you can accept functions as well as plain objects to your props:

if (props.name instanceof Function)
  props.name = props.name(dataSource) // or some other param

This way you can easily offload the details to the consumer instead of keeping everything in one place.

Then, for the endpoints, why should the logic reside in the table component? Keep it purely visual and let the parent pass down the details. In React we keep visuals and business logic in separate components so you can easily reuse the view elsewhere in the application.

Related