Pregunta

If I want to test multiple values of an enum using a case statement, and 80% of the case statements require two different if checks, is it considered poor coding to reuse that code over and over?

I actually used ctrl+c and ctrl+v and felt like the code gods would kill me.

Here is some perspective:

switch(value) {

   case value1:
   {
      if(something) { //do something; }

      if(somethingElse) { // do something else; }

      //unique for value1
   }
   break;

   case value2:
   {
      //unique for value2
   }
   break;

   case value3:
   {
      if(something) { //do something; }

      if(somethingElse) { // do something else; }

      //unique for value3
   }
   break;

   case value4:
   {
      if(something) { //do something; }

      if(somethingElse) { // do something else; }

      //unique for value4
   }
   break;

   case value5:
   {
      //unique for value5
   }
   break;

   default:
   break;

My value is randomly generated from the enum and is called a random amount of times. The goal is for value to be any random 'value' and totally independent of other cases.

¿Fue útil?

Solución

You might want to put this duplicate code into a method.

public void yourFunctionCall() {
    //Could even pass the value if needed
    if(something) { //do something; }

    if(somethingElse) { // do something else; }
}

Then call this method in your case:

switch(value) {

case value1: {
    yourFunctionCall();
    //or yourFunctionCall(value1);
    //unique for value1
} //etc..

Otros consejos

If this a reusable piece of code, you're better off turning it into a method. If not, you could simply add another switch case covering the common code using fall-through as:

switch (value) {

   case value1:

   case value3: // using fall-through

   case value4:
   {
      if (something) { /* do something; */ }

      if (somethingElse) { /* do something else; */ }
   }
}

switch (value) {

   case value1:
   {
      // unique for value1
      break;
   }

   case value2:
   {
      // unique for value2
      break;
   }

   // other unique cases
}

Using a function is probably better, but here's another way:

case value1:
case value3:
case value4:
  if(something) { /* do something */ }

  if(somethingElse) { /* do something else */ }

  if (value1)
  {
    //unique for value1
  }
  else if (value3)
  {
    //unique for value3
  }
  else // if (value4)
  {
    //unique for value4
  }
  break;

case value2:
  ...

Or with nested switch:

case value1:
case value3:
case value4:
  if(something) { /* do something */ }

  if(somethingElse) { /* do something else */ }

  switch(value)
  {
    case value1: /* unique for value1 */ break;
    case value3: /* unique for value3 */ break;
    case value4: /* unique for value4 */ break;
  }
  break;

case value2:
  ...

Use with care, I wouldn't really recommend either for production code.

Licenciado bajo: CC-BY-SA con atribución
No afiliado a StackOverflow
scroll top