6

Consider I'm writing a mobile app with user-login feature, using a framework which can be simplified like that:

class UserData{
    static token="";
    static name="";
    static balance=0;
}

class Main{
    constructor(){
        this.loginPage=new LoginPage();
        this.welcomePage=new WelcomePage();
        .
        .
        .
        if(notLogin){
            this.loginPage.show();
        }
    }
}

login page to modify UserData

class LoginPage{
    onLoginResponse(response){
        UserData.token=response.token;
        UserData.name=response.name;
        UserData.balance=response.balance;
    }
}

some other page:

class WelcomePage{
    showAboutDialog(){ 
      new AboutDialog().show();
    }
}

a general dialog may be used by other page:

class AboutDialog{
}

One day, AboutDialog needs UserData.token to submit some information to server, so I add UserData.token to AboutDialog:

class AboutDialog{
    onSubmit(){
      HttpUtils.get("(some url)/?token="+UserData.token");
    }
}

But according to Why is Global State so Evil?, I know I should not use global state, so I modify the code to use dependency injection:

class UserData{
    constructor(){
      this.token="";
      this.name="";
      this.balance=0;
    }
}

class Main{
    constructor(){
        this.userData=new UserData();
        this.loginPage=new LoginPage(this.userData);
        this.welcomePage=new WelcomePage(this.userData);
        .
        .
        .
    }
}

class LoginPage{
    constructor(userData){
      this.userData=userData;
    }
    onLoginResponse(response){
        this.userData.token=response.token;
        this.userData.name=response.name;
        this.userData.balance=response.balance;
    }
}

class AboutDialog{
    constructor(userData){
      this.userData=userData;
    }

    onSubmit(){
      HttpUtils.get("(some url)/?token="+this.userData.token");
    }
}

the problem comes when modifying WelcomePage:

class WelcomePage{
    constructor(userData){
      this.userData=userData;
    }
    showAboutDialog(){ 
      new AboutDialog(this.userData).show();
    }
}

With global state, WelcomePage doesn't need to modify and even doesn't depend on UserData, but the one without global state (or say the "dependency injection version") needs to do so. Isn't the dependency injection one violate "open-closed principle"? Isn't disallowing global state results in more coupling here?

Nevertheless, in this case, the route from Main to AboutDialog is relatively simple: Main->WelcomePage->AboutPage, what if there is a class needs more intermediate to pass UserData for it (eg: Main->SomePage->SomeSubPage->SomeDialog->SomeService...)?

Also I found the "dependency injection" one is harder to maintain for other reasons:

  1. It contains more code (eg: extra constructor and class property to pass data)

  2. When a class doesn't need UserData, I may forget to remove those constructor and class property from the intermediate(eg:WelcomePage), result in modifying UserData at intermediate accidentally, which suffers from the same problem with global state that someone may modify the data accidently

And I think the other reasons to disallow global state is not applicable for me here:

  1. concurrency problem: this app should have no multithreading codes (at far as I known)

  2. performance: assigning a data field shouldn't cost to much time

So my question is, is this case a valid reason to allow global state instead of dependency injection?

  • 1
    It isn't entirely clear to me how your question relates to the open/closed principle but you can remove your coupling of WelcomePage to userData by injecting the AboutDialog (or a factory/strategy to create the AboutDialog) into WelcomePage instead of injecting the userData. – Ant P Nov 27 '19 at 11:01
  • 7
    "Isn't disallowing global state results in more coupling here?" No, there is exactly the same amount of coupling. But with DI you can see it – Caleth Nov 27 '19 at 13:16
  • 2
    Instead of proposing a cage match between competing software principles, ask yourself, "What makes the most sense to do here? – Robert Harvey Nov 27 '19 at 15:10
7

The fallacy about the OCP in this example becomes pretty clear when you try to put WelcomePage into a reusable black-box library which is "closed against modification", and now try to change the behaviour of it in the described way without changing the code in that library.

Since the code in WelcomePage contains a statement new AboutDialog(), the library must also contain the source code of AboutDialog (or depend on some other lib with that source code), which is not closed against modification (since using UserData.token requires a change in the code there, regardless of UserData being global or not global). So WelcomePage was never following the OCP in the first place!!. Following the OCP requires a component not being tightly coupled to other components which don't follow the principle. And using global state here does not change this, so you cannot use the OCP as argument for justifying global state.

A solution to this could be to provide an interface IAboutDialog and inject an object of this type into WelcomePage at construction. Then WelcomePage can be part of a black-box library, and it will be possible to exchange the original AboutDialog by another implementation afterwards, without changing the code in the library, following the OCP. So using dependency injection correctly is exactly what makes WelcomePage follow the OCP.

To your question about coupling: when WelcomePage depends on AboutDialog, and AboutDialog on some global variable, WelcomePage depends transitively on that same global variable as well. So the effective coupling stays the same, there is not less or more coupling in the global or non-global variants. The coupling is indeed more visible in the non-global variant, but that is a different thing.

Let me finally add something trying to answer the "question I see behind your literal question": what you observed here is that introducing global state may allow sometimes to change things in a central place with less changes to code in other places (which has nothing to do with the OCP!). The answer is indeed: yes, in some exceptional cases, this can be a reason for introducing a global variable, but you have to be extremely cautious about this, make sure you know what you are doing, and that the risk of hidden dependencies is really worth the safed effort.

In your example, using a global variable will forbid your program to ever have instances of WelcomeForm and AboutBox with different user data at the same time. This may actually precisely what you need in the specific context, or it may introduce a real maintenance headache when you later find out you need it differently. So better be careful with this approach.

| improve this answer | |
4

A multi threaded environment is not required for shared mutable state to make you miserable.

Shared mutable state means you have to worry about timing. Worrying about timing sucks.

It is much nicer to know that something is the same something it was when you built it.

that's why this:

class LoginPage{
    constructor(userData){
      this.userData=userData;
    }
    ...
}

bothers me. loginPage holds a reference to the publicly available userData. userDatacan change whenever something writes to it. Yuck. That means loginPage can change whenever someone writes to userData. That's confusing.

I'd rather see this:

class LoginPage{
    constructor(userData){
        this.token=userData.token;
        this.name=userData.name;
        this.balance=userData.balance;
    }
    ...
}

Now loginPage has it's own state that it can encapsulate. It can hide it's data and keep it from changing. It can also decide to spin up another copy of userData and fling it at some boundary when it's ready. But it doesn't have to worry that everything that touched userData is still messing with it.

This is called a defensive copy. It may seem like extra work but it is a god send when you're staring at some weird value in a debugger and wondering how and when it got this way.

If you're looking at this and thinking that's terribly coupled. I don't want to do this between every object. You're absolutely right. That's why you only do this with DTO's. I'm treating UserData as a Data Transfer Object. These are just data structures wrapped in getters and setters (which are mostly for debugging). Ideally your program isn't just flinging DTO's around. Save DTO's for crossing a significant boundary. Like a GUI, DB, FileSystem, Network, etc. Within your program you don't share state. You move methods to live with the state they need. You polymorphically pass messages between your proper objects. Rather then ask each other questions they tell each other what to do.

Do that and you have much less coupling. The state lives in one place where you can tell it to do things.

If you're going to do Dependency Injection do it all the way before you decide it's a waste. DI done halfway is just a bigger mess.

| improve this answer | |
  • It seems to me that the whole point of the example was that information from the login process needs to be used in the About dialog. Storing the data locally on the LoginPage makes that not work at all. – Winston Ewert Nov 27 '19 at 16:43
  • Or to take it a step further, that constructor should take token, name, and balance rather than getting them from userData. – StackOverthrow Nov 27 '19 at 19:08
  • @NoU up to a point. See Introduce Parameter Object – candied_orange Nov 27 '19 at 19:13
  • @candied_orange Indeed. As your link says, it depends on how closely related the parameters are. The asker's UserData class may already be guilty of being a bag of random junk, since token sounds like something transient while name sounds like a property of the user. – StackOverthrow Nov 27 '19 at 19:21
1

The situation is actually even "worse" than what you already know, because the proposed solution with "dependency injection" is arguably still a global state. Yes, it is marginally better, because you explicitly pass in the state, but you still use the same state as data everywhere.

To put it differently, the problem with global state is essentially that it is unmaintainable. You use/modify something which might be used/modified somewhere else in some unpredictable or non-transparent ways. Is this problem solved by passing the same UserData instance around? I think it would be pretty hard to argue that it is solved.

Object-orientation in particular (and I just assume now that you want to use object-orientation) solves this problem by proposing that state can not be global at all, what's more it must be local to the object. I.e. objects have a state and all operations that use that state.

In other words, you can pass an object around, but you can not pass data around. So instead of UserData, you could have a User, and that user could contain all the operations you want to do, like making that HTTP request or just adding a token to a prepared HTTP request, etc.

So no, this is not a valid case for a global state, and in addition your proposed solution is not going far enough to really avoid the problems associated with it.

| improve this answer | |
0

This is just an answer on top of other answers and probably doesn't answer your question.

Advantage of global variables in this example:

  • WelcomePage does not need to know about whether any of its components need certain variables such as user data

Advantage of dependency injection:

  • Solves issues with global variables such as:
    • Variables not being part of API of code using it (which solves problems such as making unit testing easier, and making the code purer)
    • User data can change in different scopes in your application, instead of one single global variable. Very important to prevent what I call "data leaks"[1], and very useful in certain situations

Is there anything in between those two options?

One solution (which is now used frequently in React) is a context management system. Context variables are like global variables, except that their values are set for a specific scope/layer and do not automatically affect the outer scope. They will typically be part of the API of the code using the variables, but will not of in-between layers such as WelcomePage in your example.

The variables are automatically passed through all your functions and classes, and must be optional as they are not guaranteed to be defined (for example if you run your WelcomePage alone without any other code using it, WelcomePage does not define any user data variable).

Java, and I think all programming languages, don't have any built in context system. You could always create a singleton for just your own context system, or if you are happy to pass it through all your functions you may do that too. There may be context system libraries out there already, but this is not a Java-specific answer.


[1] "data leaks" - when data is made available to or retrieved from parts of the application that should not be sharing the same variable (typically caused by using single global variables). To prevent this you must add to ability to create separate layers or scopes in your application. My great example for this is server-side-rendering for webpages and client-hydration. When this is done, the client and the server will run the same code. If you have a user data global variable, the server may leak that to other clients, since the entire server will be accessing the one user data variable. A layering/scoping system would prevent this.

| improve this answer | |
-3

It doesn't really matter whether your global state is accessed by a global variable, by a singleton, or by passing pointers to it everywhere. It has the mostly the same disadvantages regardless of how it is accessed. Any sort of shared state, regardless of the mechanism, causes pretty much the same problems.

But we can't eliminate shared state. There is invariably some amount of application-wide state. I think the best way forward is simply to be honest and explicit about that. Having a well-defined collection of global state is much better then widely distributed pieces of shared state.

What I would do for a mobile application is define an ApplicationState object which holds the state of the application. It would hold a reference to your UserData and anything else of that nature. Then it becomes very clear what shared state I have, and when I am reading from and modifying it.

| improve this answer | |
  • 2
    (not the downvoter) While it's true that shared state is unavoidable at the highest level of any non-trivial application, putting it in a globally accessible ApplicationState object would encourage violation of the Law of Demeter. The highest level of the application should inject the lower level objects with whatever state they need. – StackOverthrow Nov 27 '19 at 19:15
  • @NoU, I used to think that, but I've concluded that it isn't true. You are much better off having a single well-defined collection of global state then injecting lots of pieces of state into different things. – Winston Ewert Nov 27 '19 at 20:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.