3

I have a trivial, but irritating problem in Java. Suppose we have the following class and method:

class A{
    void doSth(int[] array){    
        int index1, index2, index3;
        int value1, value2, value3;    

        if(array[index1] > 10){    
            //Long code modifies value1, value2, value3
        }  

        if(array[index3] > 100){    
            //Same long code modifies value1, value2, value3
        }      

        if(array[index2] > 20){    
            //Same long code modifies value1, value2, value3
        }    
    }

Disregarding what this is trying to achieve, I would like to somehow make this redundancies disappear. usually, I would pass the values to a hlper method, but I can't, since the block is modifying local variables. Any idea how to simplify this?

Improve this question
3
  • @Wojtek Maybe read more than the first sentence of a question? The whole question is about how he can overcome the problems with exactly that approach. And really in java there no good answers to that (pass class/array of items around, return class/object and reassign - both ugly) Commented Jun 10, 2012 at 19:15
  • How do you use value1-value2-value3 later? Commented Jun 10, 2012 at 19:17
  • Why not? if (array[index1] > 10 || array[index3] > 100 || array[index2] > 20) ? Does the sequence of modifications matter? Commented Jun 10, 2012 at 19:34

3 Answers 3

Reset to default
9

It sounds like you value1, value2 and value3 probably have some meaning in combination. So encapsulate them into a separate class, and at that point you can call a method which either modifies an existing instance or returns a new instance of that class. Either way, with a single local variable you'll be fine.

Improve this answer
Sign up to request clarification or add additional context in comments.

5 Comments

Well, these values are local measures, very specific to only one specific approach within this one class... I know I could encapsualte that into a class but that seems like a bit of an overkill as these values could change in the next class. My question is more broad - can we escape from Objecting in this case?
@Bober02: Not easily, no. I don't see why you'd want to - it sounds like they are related values, so encapsulating them makes sense. You can always turn them into a private nested (probably static) class to reduce the scope.
hmmm... That sounds nice indeed. Private static is something I did not consider. Thanks!
Done that in the past, but if you really do this universally you end up with hundreds of special purpose classes. And sometimes it's really not two or more coherent values, but more one is a byproduct of the other. E.g. parseInt - if we couldn't throw an exception (which isn't that great design wise anyhow), we'd either have to parse everything twice or use a class that contains bool parseable, int value. out parameters as c# or a simple way to pass tuples around (python) would be a useful addition in my opinion.
@Voo: If you end up doing this a huge amount, that would suggest to me that the design might not be right. I can't say it's a need I come across particularly regularly - certainly not enough to make the extra classes a problem.
0

You can refactor your code to break out a method into which you pass the everything you need, but you'll need to introduce some private fields.:

   private int value1, value2, value3;

private void doIt(int index, int threshold) {
    if (array[index] <= threshold)
        return;
    ... //Same long code modifies value1, value2, value3
}

then replace your main code to this:

void doSth(int[] array, int index1, int index2, int index3) { 
    doIt(index1, 10);
    doIt(index3, 100);
    doIt(index2, 20);
}

And you're done.

Improve this answer

2 Comments

values are NOT fields of a class!
@Bober02 Just noticed that - I edited to make them so. I still think this is a case for refactoring
0

If the codes in the if statements are all exactly the same, why don't you just do it like this:

class A{
   void doSth(int[] array){
      int value1, value2, value3;
      int index[][] = {
        {val1, 10},
        {val2, 100},
        {val3, 20}
      };

      // ... 

      for(int i = 0; i < index.length; i++){
         if(array[index[i][0]] > index[i][1]){
           // ... Long code modifies value1, value2, value3
         }
      }
   }
}
Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.