How can I avoid code duplication in C# while and do-while loops?

Question

I have a loop inside a C# method that has the following structure.

do
{
     getUserInput();
     if (inputIsBad)
     {
          doSomethingElse();
     } 
} while (inputIsBad);

alternately, with a while loop:

getUserInput();
while (inputIsBad)
{
     doSomethingElse();
     getUserInput();
}

But both methods use redundant code: the do-while has both an if statement and while loop checking the same condition; the while loop calls getUserInput() both before and inside the loop.

Is there a simple, non-redundant, non-ad hoc way to do what these method patterns do, either generally or in C# specifically, that only involves writing each basic component once?

Possibly: `X v; while ((v = getUserInput()) != null /* assumes sentinel for boolean */ && isInputBad(v)) { doSomethingElse(v); }`. It looks better (to me) when the `&&` starts a new line. Remember that the `&&` operator uses short-circuit evaluation. Values of `v` added to usages, as common-case, and can be eliminated (including assignment). — user2864740, Jan 26 '20 at 00:49
Or something like `while(isInputBad(v = getUserInput()) { doSomethingElse(v); }`, although I find such "too tricky" myself. — user2864740, Jan 26 '20 at 00:51
This is a good question. I've hit this scenario a handful of times over the years and I don't recall an elegant solution that meets your criteria and is easy to read at a glance. I'd opt for your second example — Jonesopolis, Jan 26 '20 at 00:58

user2864740 · Answer 1 · 2020-01-26T02:42:46.463

Assuming that getUserInput(..) can be converted into a expression yielding a boolean value*..

while (getUserInput()
    && isBadInput()) {
  doSomethingElse();
}

// Prompts for user input, returns false on a user-abort (^C)
private bool getUserInput() { .. }

Other variations (presumed without non-local state) shown in comments.

*Trivially, it can always be written as a wrapping function - see Local Functions, introduced in C#7. (There are other methods for the same effect, some of which I consider 'too clever'.)

// local function
bool getUserInputAlwaysTrue() {
   getUserInput(); // assume void return
   return true;
}

while (getUserInputAlwaysTrue()
    && isBadInput()) {
  doSomethingElse();
}

This can be followed to pushing out the logic further, in some cases. The general premise holds: getUserInput() is always invoked prior to the next isBadInput().

// local function or member method
// Prompt for user input, returning true on bad input.
bool getCheckedUserInput() {
   getUserInput(); // assume void return
   return isBadInput();
}

while (getCheckedUserInput()) {
  doSomethingElse();
}

score 2 · Answer 2 · answered Jan 26 '20 at 23:16

2

do
{
     getUserInput();

     if (!inputIsBad) break;

     doSomethingElse();

} while (true);

answered Jan 26 '20 at 23:16

Kahou

3,200
1
14
21

1

As much as I find sentinels _ad hoc_, this general kind of solution is simple, easy to read, and removes the code duplication in the original post. – Jacob Archambault Jan 31 '20 at 15:00
1

One minor quibble: setting the while to true actually makes the 'do' unnecessary, so this can be simplified further by converting it to a while loop. – Jacob Archambault Jan 31 '20 at 15:24

score 0 · Answer 3 · answered Jan 26 '20 at 21:44

0

I would use a boolean variable, which you need to declare outside the body of the loop. That way you only need to run the inputIsBad check once. I have turned it into a method as well, since that seems more logical.

bool badInput = true;  // Assume bad until checked -- failsafe.
do
{
  getUserInput();
  badInput = inputIsBad();
  if (badInput)
  {
    doSomethingElse();
  } 
} while (badInput);

answered Jan 26 '20 at 21:44

rossum

15,344
1
24
38

This contains the same redundancy as the version first stated in the original question while adding unnecessary code: the boolean is still being checked by both the if-statement and the while loop's conditions, while the initial assignment to badInput is never used (since the variable is reassigned in a do-while loop before being checked). – Jacob Archambault Jan 31 '20 at 14:50
This version only runs `inputIsBad()` once. That presumably takes longer than a simple boolean check. You initial version ran it twice. – rossum Jan 31 '20 at 15:46

score 0 · Answer 4 · answered Jan 31 '20 at 16:12

Building on user2864740's answer:

Assume getUserInput() can be converted into a function which returns true if the input is good and bad otherwise. Assuming its original return type wasn't boolean or void, return its original return value via an out or ref parameter depending on the case, e.g.

int originalReturnValue;

while (!getUserInput(out originalReturnValue))
{
     doSomethingElse();
} 

...

bool getUserInput<T>(out T output)
{
// method body
}

How can I avoid code duplication in C# while and do-while loops?

4 Answers4