Is there some way I can optimize this C# code?

Question

I have my code and I would like to optimize it. For me it looks like it's already optimized. Can anyone suggest how I could make it a bit more optimized?

if (target == "power")
{
    return new JsonResult { Data = new { RC = new Data.AdminPower(datastoreValue).Refresh() } };
} 
if (target == "notes")
{
    return new JsonResult { Data = new { RC = new Data.AdminNotes(datastoreValue).Refresh() } };
}
if (target == "book")
{
    return new JsonResult { Data = new { RC = new Data.AdminBook(datastoreValue).Refresh() } };
}
return null;

Since it already looks good enough to you, you can just move on in most cases. — BoltClock, Jul 28 '11 at 00:01
Surely any performance you gain here would be fairly trivial? But if you really want to change something, I would agree with using switch. — user122211, Jul 28 '11 at 00:04
Can you turn your target into an enumeration? Then you could have `Target.Book`, `Target.Notes`, etc. Then your comparing enumerated values, not literal strings. However, I don't see where `target` is being assigned, so that may not be an option. — George Johnston, Jul 28 '11 at 00:13

score 2 · Answer 1 · answered Jul 28 '11 at 00:01

2

If you know that your method will be called more often with "book" values, then you should put that first. Basically sort by order of frequency.

answered Jul 28 '11 at 00:01

CodeNaked

40,753
6
122
148

Nick Rolando · Accepted Answer · 2011-07-28T00:10:20.673

2

switch statements are great for these situations:

switch(target)
{
case "power":
  return new JsonResult { Data = new { RC = new Data.AdminPower(datastoreValue).Refresh() } };
case "notes":
  return new JsonResult { Data = new { RC = new Data.AdminNotes(datastoreValue).Refresh() } };
case "book":
  return new JsonResult { Data = new { RC = new Data.AdminBook(datastoreValue).Refresh() } };
default:
  return null;
}

Dont really need the breaks tho, since it will be returning each time, but its good practice..

edited Jul 28 '11 at 00:10

answered Jul 28 '11 at 00:02

Nick Rolando

25,879
13
79
119

1

See http://stackoverflow.com/questions/427760/when-to-use-if-else-if-else-over-switch-statments-and-vice-versa – CodeNaked Jul 28 '11 at 00:03
3

@Nick You're going to get `unreachable code` warnings for the `breaks;` – George Johnston Jul 28 '11 at 00:04
I don't think the breaks are necessary. – Eric Jul 28 '11 at 00:06
I said the breaks weren't necessary, just said its good practice imo. OP can delete if she wants – Nick Rolando Jul 28 '11 at 00:07

Evan Mulawski · Answer 3 · 2011-07-28T00:18:44.130

0

The only thing I would do is change the ifs to an if-elseif chain. There isn't really anything else you can do to improve performance.

edited Jul 28 '11 at 00:18

answered Jul 28 '11 at 00:01

Evan Mulawski

54,662
15
117
144

The OP returns in the if-block, so `else if` won't help. – CodeNaked Jul 28 '11 at 00:02
@CodeNaked: I didn't say it would help. It is better communicated to anyone reading the code that the conditions are mutually exclusive. – Evan Mulawski Jul 28 '11 at 00:05

score 0 · Answer 4 · edited Jul 28 '11 at 00:17

0

I don't know what's your idea about optimized, as a matter of speed, just put some 'else' before second and third 'if's but if you mean less code lines, then may be something like this could help you:

return 
((target == "power") ? new JsonResult { Data = new { RC = new Data.AdminPower(datastoreValue).Refresh() } } :
((target == "notes") ? new JsonResult { Data = new { RC = new Data.AdminNotes(datastoreValue).Refresh() } } : 
((target == "book") ? new JsonResult { Data = new { RC = new Data.AdminBook(datastoreValue).Refresh() } } : null))))

edited Jul 28 '11 at 00:17

Evan Mulawski

54,662
15
117
144

answered Jul 28 '11 at 00:06

Afshin

1,222
11
29

5

My eyes would roll up into my head if I saw that line in code I had to maintain, and I'd probably mutter some very unnice things about the programmer's origin. Optimization is fine, but not to the point where readability goes out the window.... – Tim Jul 28 '11 at 00:10
2

While you're at it, remove all of the spaces and line breaks :). Nothing like reading a multiline ternary statement to wake you up in the morning. – Evan Mulawski Jul 28 '11 at 00:13

score 0 · Answer 5 · answered Jul 28 '11 at 00:08

As someone else mentioned, you should put them in order of most likely to occur to minimize your total number of incorrect comparisons.

Maybe it's possible to change "power" "notes" and "book" to an enum or int representation, and that may be slightly faster than the string comparisons.

There's not a whole lot you can do that will result in any significant optimizations, though.

Is there some way I can optimize this C# code?

5 Answers5