3

GOAL: to have a main class which has some global variables which need to be modified by other classes.

I have a class with a structure like below

    public class ClassName {
       public static int variable;

       public void meth1(){
       }    
       ...
   }

Then I have other classes which need to use and update that variable. The structure of these classes is like this:

public class ClassName2 {
     public void meth(){
           ClassName1.variable = 4;
      }
}

QUESTION: how is bad this approach? I was asking myself what happens if I want to use ClassName2 in another project? And answering to myself I've discovered that I've created a dependency of 'ClassName2' from 'ClassName1' How can I avoid this dependency from 'ClassName1' ?

Maicake
  • 225
  • 2
  • 7
  • 2
    Possible duplicate of [Is it bad practice to use public fields?](https://softwareengineering.stackexchange.com/questions/161303/is-it-bad-practice-to-use-public-fields) – gnat Sep 18 '18 at 11:21
  • 2
    @gnat, public fields are not the same as public static fields/variables. – David Arno Sep 18 '18 at 11:37
  • 3
    It is trivally easy for you to find answers to this question by simply search on the search engine of your choice for "why are global variables bad?" – David Arno Sep 18 '18 at 11:38
  • 1
    @DavidArno for the context of this question, these are essentially the same - the only difference of using static accessor methods is insubstantial here – gnat Sep 18 '18 at 11:40

2 Answers2

4

Global variables are never necessary unless you are interfacing with existing code that imposes unsuitable constraints.

One alternative is to make the dependency explicit:

1. Create a class that describes the purpose of this shared data. Without knowing your problem domain, I'm going to call it a Counter.

class Counter {
  private int value = 0;

  public void incrementBy(int x) {
    if (x < 0)
      throw IllegalArgumentException("must increment by non-negative value");
    value += x;
  }

  public int get() { return value; }
}

Note that wrapping the variable through accessor functions allows us to impose checks and constraints. This lack of checks is one major problem with exposing variables directly.

2. Use some dependency injection mechanism to supply a Counter instance to all classes that need it. For example, we can use constructor injection:

public class ClassName2 {
  private Counter counter;

  public ClassName2(Counter counter) {
    this.counter = counter;
  }

  public void meth(){
    counter.incrementBy(4);
  }
}

The counter is created in some initialization code (like your main() function) and then provided when the dependent classes are instantiated:

Counter counter = new Counter();
ClassName  instance  = new ClassName(counter);
ClassName2 instance2 = new ClassName2(counter);

All global variables have a similar instantiation, but it's just done implicitly during class loading. By managing the initialization ourselves we gain a lot of flexibility:

  • we can supply a different Counter class during testing.
  • we are able use multiple Counter instances if different parts of the code are not supposed to share a Counter, for example when the original ClassName functionality is reused in a different context.

If you cannot make these dependencies explicit, at least encapsulate the variable using static methods in a similar fashion.

amon
  • 132,749
  • 27
  • 279
  • 375
  • 2
    This answer mentions something between the lines which I found to be one of the most important pieces of know-how gained through experience: _If you can't decide between class A and class B, the correct answer is probably class C_. Class C being the `Counter` class in your example. – R. Schmitz Sep 18 '18 at 11:48
  • 1
    I second that. There are two cases where this appears often: one is a circular dependency, and the other is when you can't decide where to put behavior. In both cases, there are again two possible solutions: one, there is a third component missing, or two (the opposite), they are actually one component. – Jörg W Mittag Sep 18 '18 at 11:59
  • shouldn't be get return value instead of x? – Maicake Sep 18 '18 at 12:36
  • @amon does this apply also with android MainActivity class for example? – Maicake Sep 18 '18 at 13:18
  • 1
    @Maicake probably not, or at least not as strongly. I'm not super familiar with Android development. If Android enforces some singleton instance, then it might be more convenient to use that singleton to store global data and not make the dependencies more explicit. However, in a nontrivial app you will likely want to keep Android-specific parts out of your business logic. And you might still want to encapsulate the global data through methods. – amon Sep 18 '18 at 13:26
-2

The bad thing is not that C2 depends on C1. Reusing classes is a good thing. If it makes sense to define the task of one class partially in terms of the task of another, then by all means do so. The point about exposing public state from a class is that it undermines the advantage of reuse and encapsulation.

Example: If you have a Fridge class, you want it to take care of your perishables, and ideally the fridge would be smart enough to set a reasonable temperature by itself, rather than have you micromanaging it. A good set-up would be to hand your Fridge either an IceCream or a Cucumber object, and let the fridge decide automatically how cold the contents need to be kept. Putting in a Cucumber and then setting the temperature manually to 4 is more work and introduces a possibility for error where the type of food and the temperature value don't match. If there is to be knowledge in your system about how to refrigerate different foodstuffs, then it should reside in the Fridge class and not separately in every user of that class.

Kilian Foth
  • 107,706
  • 45
  • 295
  • 310