Domanda

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.

È stato utile?

Soluzione

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..

Altri suggerimenti

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.

Autorizzato sotto: CC-BY-SA insieme a attribuzione
Non affiliato a StackOverflow
scroll top