CODEWITHSUNDEEP
c#if-statementrefactoring
grady
grady

Reputation: 12785

Simplify if condition?

I have this code:

int someValue = 100;
if (x == 5)
{
    if (someCondition)
    {
        return someValue;
    }
    return someValue / 12;
}

if (x == 6)
{
     if (someCondition)
     {
         return someValue * 12;
     }
    return someValue;
}

As you see, someCondition is always the same, just the returned value differs. Is there a way to simplify this some more?

Upvotes: 4

Views: 9567

Answers (11)

iEfimoff
iEfimoff

Reputation: 372

Use an object instead of a switch, added "adopting" tests.

const state = {
  5: {
    true: (someValue) => someValue,
    false: (someValue) => someValue / 12
  },
  6: {
    true: (someValue) => someValue * 12,
    false: (someValue) => someValue
  }
}

const test_values = [0, -1 , 1, 12, -12, 1.2, -1.2]

test_values.map(value => Object.keys(state).map(x => Object.keys(state[x]).map(condition => {
  console.log(`value = ${value}, x = ${x}, result = ${state[x][condition](value)}`)
})))

Upvotes: 0

Bazzz
Bazzz

Reputation: 26942

Let's see, what do you think of this?

int someValue = 100;

if (x == 5)
 return someCondition ? someValue : (someValue / 12);
else if (x == 6)
 return someCondition ? (someValue * 12) : someValue;

Upvotes: 7

iEfimoff
iEfimoff

Reputation: 372

switch(x) {
    case 5: return someCondition ? someValue : someValue / 12;
    case 6: return someCondition ? someValue * 12 : someValue;
}

Upvotes: 1

Fred
Fred

Reputation: 31

A single line of code:

condition ? someValue * ((x == 6) ? 12 : 1) : someValue / ((x == 5) ? 12 : 1);

Is this simple? I think so.
Is this easy to read? At some extent.
Is this good? I wouldn't say so. (+ the generated IL is slightly different)

Upvotes: 2

Tony Hopkinson
Tony Hopkinson

Reputation: 20330

Much of a muchness really, how about

Double scalar = 12;
switch(x)
{
  case 5 : 
    scalar = 1f/12;
    break;
  case 6 : 
    break;
  default : 
    return 100;
}
if (someCondition)     
{         
  return someValue;     
}     
return someValue * scalar;

Upvotes: 1

Guffa
Guffa

Reputation: 700840

You can switch the if statements, so that you only repeat the condition once. You can use a switch for checking the x value:

int someValue = 100;
if (someCondition) {
  switch (x) {
    case 5: return someValue;
    case 6: return someValue * 12;
  }
} else {
  switch (x) {
    case 5: return someValue / 12;
    case 6: return someValue;
  }
}

Upvotes: 1

Shadow Wizzard
Shadow Wizzard

Reputation: 66397

This has no "nested" statements so looks cleaner to me:

int someValue = 100;

if ((x == 5 && someCondition) || (x == 6 && !someCondition))
    return someValue;

if (x == 5)
    return someValue / 12;

if (x == 6)
    return someValue * 12;

Upvotes: 3

Chowlett
Chowlett

Reputation: 46685

You've got two variables (x and someCondition), and 3 distinct outcomes; so, yes, you can do better than two pairs of two nested tests. The best you can do is:

if (((x == 5) && someCondition)) ||
    ((x == 6) && !someCondition)))
{
  return someValue;
}
else if (x == 5)
{
  return someValue / 12;
}
else if (x == 6)
{
  return someValue * 12;
}

Upvotes: 2

Rich O'Kelly
Rich O'Kelly

Reputation: 41767

It depends on what you mean by simplify...

The following has fewer lines of code and doesn't sacrifice anything in terms of readability (IMO):

var someValue = 100;
switch (x)
{
  case 5:
    return someCondition ? someValue : someValue / 12;
  case 6:
    return someCondition ? someValue * 12 : someValue;
  default:
    return someValue;
}

Upvotes: 4

Gabriel
Gabriel

Reputation: 3045

Personally I would use a switch statement instead of two if statements. Or you could make a function where you pass the x * 12 or x / 12...

Upvotes: 0

user596075
user596075

Reputation:

You have different ways to handle the result of the same conditional statement. Because of this, it is probably more efficient (and easier on the eyes) to keep it the way you have it.

If your return expressions were the same for each conditional statement then I would revise the code, but each block of code has a different return value and because of that makes it unique.

So no, there really isn't a way to simplify what you've done.

Upvotes: 2

Related Questions