0

I have this simplified code, quite simple:

export const OwnRedirect = () => {
  const { pipelineType, ticketId, productId } = useParams<{
    pipelineType?: string;
    ticketId?: string;
    productId?: string;
  }>();
  let path = '';
  if (pipelineType) {
    path = `/pipeline/${pipelineType}`;
  } else if (ticketId) {
    path = `/ticket/${ticketId}`;
  } else if (productId) {
    path = `/product/${productId}`;
  } else {
    path = '/pipeline/local';
  }
  return <Redirect to={path}/>;
};

But I found it not readable enough. Anybody has an idea how to refactor this code not using if or switch or let or nested ternary operator?

2
  • Providing the most minimal/simple instance of your problem enhances the chance to get good answers. Commented Jun 20, 2020 at 10:27
  • Already edited. Commented Jun 20, 2020 at 12:42

3 Answers 3

1

To make it easier to read, you can take advantage of short-circuit evaluation with && and ||. To make things cleaner and easier to test, I also like creating really small functions that only do one thing, but do it well. It avoids mixing logic (which path to choose) and functionality (building those paths):

export const OwnRedirect = () => {
  const { pipelineType, ticketId, productId } = useParams<{
    pipelineType?: string;
    ticketId?: string;
    productId?: string;
  }>();
  
  const path = (
    (pipelineType && buildPipelinePath(pipelineType)) ||
    (ticketId     && buildTicketPath(ticketId)      ) ||
    (productId    && buildProductPath(productId)    ) ||
    `/pipeline/local`
  );

  return <Redirect to={path} />;
};

function buildPipelinePath(pipelineType) {
  return `/pipeline/${Number(pipelineType) === 1 ? 'local' : 'global'}`;
}

function buildTicketPath(ticketId) {
  return `/ticket/${ticketId}`;
}

function buildProductPath(productId) {
  return `/product/${productId}`;
}
Sign up to request clarification or add additional context in comments.

Comments

0

sometimes just separating it into a function helps with readability.

const toUrl = ({ pipelineType = '', ticketId = '', productId = '' }) => {
  if (pipelineType) {
    return `/pipeline/${pipelineType}`;
  } 
  
  if (ticketId) {
    return `/ticket/${ticketId}`;
  } 
  
  if (productId) {
    return `/product/${productId}`;
  }
  
  return '/pipeline/local';
}


export const OwnRedirect = () => {
  const params = useParams<{
    pipelineType?: string;
    ticketId?: string;
    productId?: string;
  }>();
  
  return <Redirect to={toUrl(params)}/>;
};

Comments

0

You may use a tagged sum. For example:

// Generic functions
// B is the function compose combinator
const B = g => f => x => g (f (x))

const taggedSum = xs => Object.fromEntries (
  xs.map(tag => [tag, value => ({ tag, value })])
)

const cata = matches => ({ tag, value }) => matches[tag] (value)
/////////////////////////

const Kind = taggedSum (['pipeline', 'ticket', 'product', 'local' ])

const path = cata ({
    pipeline: x => `/pipeline/${x}`,
    ticket: x => `/ticket/${x}`,
    product: x => `/product/${x}`,
    local: () => `/pipeline/local`
})

const output1 = B (path) (Kind.pipeline) (1)
const output2 = B (path) (Kind.ticket) (14)
const output3 = B (path) (Kind.product) (3421)
const output4 = B (path) (Kind.local) (0)


console.log ('pipeline:', output1)
console.log ('ticket:', output2)
console.log ('product:', output3)
console.log ('local:', output4)

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.