12 Replies Latest reply: Mar 25, 2010 12:44 PM by jschellSomeoneStoleMyAlias RSS

    Eliminating a God Object (general how-to question)

    796262
      Hey all,

      In my program (which I inherited from somebody else who I gather was a C coder), there are a bunch of classes (110 or so) that all seem to rely on a central class.

      This central class contains the bulk of the program functionality, a bunch of global variables, and is the bridge between almost any interaction between any of the other classes. For example, things like this can be found all over the place:

      (in ClassA):
      getCentralClass().getClassC().doSomethingInC();
      This never sat well with me, and I've identified this as the [God Object|http://en.wikipedia.org/wiki/God_object] anti-pattern. I suppose recognizing the problem is the first step, but I'm having trouble coming up with a better way to do things.

      I know asking general questions like this can be pretty annoying (especially when no good example code is given), but I'd love to read more about how to get rid of anti-patterns like this. I know what the good practices are (each Object being in charge of one thing, tell-don't-ask, etc), but starting with a conglomeration of a few years' worth of adding things that work but aren't well designed is a completely different story than starting on my own program from scratch.

      So, do you have any advice (or any resources you can point me to) on eliminating a god object? I know how to prevent one when starting from scratch, but every time I try to refactor this, it seems to just get worse.
        • 1. Re: Eliminating a God Object (general how-to question)
          jduprez
          Just a quick note: pretentious as I might sound, I deem that God object is badly named: you don't care whether a single object holds a lot of the app's data, or whether most flows have to go through a single object. What has more impact on maintenability is when a single God Class(tm) holds too much behavior (business code).

          The very sketchy code you give doesn't sound like it: the CentralClass knows all other collaborators (e.g. class C), and somehow acts as a Mediator, but doesn't implement all behavior itself.
          Not so much of a Mediator actually, because a fairly designed mediator should receive a thereIsSomethingToDo() request and dispatch it to a C instance (+someC.doSomething()+) without the caller code having to know that C exists.

          So, in a shorter sentence, the Mediator pattern may be a way out, if you really thik there's a problem.

          Edited by: jduprez on Mar 18, 2010 4:20 PM
          • 2. Re: Eliminating a God Object (general how-to question)
            796262
            jduprez wrote:
            Just a quick note: pretentious as I might sound, I deem that God object is badly named: you don't care whether a single object holds a lot of the app's data, or whether most flows have to go through a single object. What has more impact on maintenability is when a single God Class(tm) holds too much behavior (business code).
            The very sketchy code you give doesn't sound like it: the CentralClass knows all other collaborators (e.g. class C), and somehow acts as a Mediator, but doesn't implement all behavior itself.
            True, and I probably should have explained that that's happening too. Basically, each non-central class is in charge of a piece of the GUI. Based on the input from that GUI, the non-central classes then invoke behaviors in the CentralClass, instead of simply invoking those behaviors themselves. Something like this:
            class B{
               JButton.actionPerformed(){
                  centralClass.buttonPressedB()
               }
            }
            
            class CentralClass{
               public void buttonPressedB(){
                  System.out.println("B was pressed!");
               }
               public void buttonPressedC()...
            }
            I know the quick answer is to simply move the business logic into whatever class invokes it, but it gets trickier because oftentimes several classes invoke a behavior based on different input. Something like this:
            class A{
               JButton.actionPerformed(){
                  centralClass.buttonPressedAorB()
               }
            }
            class B{
               JButton.actionPerformed(){
                  centralClass.buttonPressedAorB()
               }
            }
            
            class CentralClass{
               public void buttonPressedAorB(){
                  System.out.println("A or B was pressed!");
               }
               public void buttonPressedC()...
            }
            In that case, how can I move the buttonPressedAorB method out of the CentralClass? I've begun creating ActionListeners in the CentralClass and passing them in to the other classes that should use them, but that doesn't seem to be enough: the business logic is still in the CentralClass.
            Not so much of a Mediator actually, because a fairly designed mediator should receive a thereIsSomethingToDo() request and dispatch it to a C instance (+someC.doSomething()+) without the caller code having to know that C exists.

            So, in a shorter sentence, the Mediator pattern may be a way out, if you really thik there's a problem.
            I'll definitely look into it, thanks. I do think there's a problem: this CentralClass contains 83 global variables, 132 methods, and 5,698 lines of code. Seems a bit bloated!
            • 3. Re: Eliminating a God Object (general how-to question)
              800387
              You can always implement a Singleton (if it is judiciously used). Does the God object simply create one instance of each of the helper classes? If yes, you could instead refactor so when you are creating your listeners, the dependent/helper class is moved out of the God object and placed into a ctor for the listener and stored as a reference.

              - Saish
              • 4. Re: Eliminating a God Object (general how-to question)
                jduprez
                True, and I probably should have explained that that's happening too. Basically, each non-central class is in charge of a piece of the GUI. Based on the input from that GUI, the non-central classes then invoke behaviors in the CentralClass, instead of simply invoking those behaviors themselves.
                At the cost of sounding dense: if this behavior involves updating other pieces of the GUI, it may be a blessing that there is a man-in-the-middle that does know all N GUI pieces, and spares each of the N pieces to have to know the (N-1) others. That's the #1 selling argument of the Mediator pattern (#2 being, the Mediator may expose a higher-level-event-handling interface, leaving the logic agnostic of the widget-specific interfaces).
                I know the quick answer is to simply move the business logic into whatever class invokes it, but it gets trickier because oftentimes several classes invoke a behavior based on different input. Something like this:
                class A{
                JButton.actionPerformed(){
                centralClass.buttonPressedAorB()
                }
                }
                class B{
                JButton.actionPerformed(){
                centralClass.buttonPressedAorB()
                }
                }
                In that case, how can I move the buttonPressedAorB method out of the CentralClass?
                One suggestion: define an interface that exposes method buttonPressedAorB(). Make the client code A and B depend on this interface instead of the GodClass. GodClass itself will implement, temporarily, the interface, and probably several interfaces of that kind. A and B will fetch/receive and instance of the GodClass, but don't need to know.
                Then for each interface try to extract a new class that implements the interface (and nothing else).

                When that's harder (because buttonPressedAorB() uses state or variables common to other such methods), then coalesce the interfaces.
                I've begun creating ActionListeners in the CentralClass and passing them in to the other classes that should use them, but that doesn't seem to be enough: the business logic is still in the CentralClass.
                I don't think it's a good option: you may have widgets that generate other kinds of events. the adapt-ation (bingo, one more patter name :o) between the event and listener types would be awkward.
                Plus, I did that once in an app (create the listeners in a class, and add them to the widgets in another class), and the result was really messy, in particular because the listeners had to know too much of the widgets they listened to.
                I do think there's a problem: this CentralClass contains 83 global variables, 132 methods, and 5,698 lines of code. Seems a bit bloated!
                (devil's advocate) If there are 83 widgets to coordinate, someone has to know them all :o)
                More seriously, I agree that sounds much like too big of a class. Splitting it requires to identify which of those variables need coordination (are accessed together by at least one method). Note that one option to consider is, merely than one mediator, a tree of mediators (with one second-evel mediator mediating between several first-level mediators).
                • 5. Re: Eliminating a God Object (general how-to question)
                  782681
                  jduprez wrote:
                  ... If there are 83 widgets to coordinate, someone has to know them all :o)...
                  not necessarily, if these are arranged using Composite :)

                  // (looking at my screen) hm hard to imagine user capable of working with 83 GUI widgets unless these form a coarse-grained hierarchy. Menu here, toolbar there
                  • 6. Re: Eliminating a God Object (general how-to question)
                    791266
                    Hmm.. My latest Swing application had a small event/message bus, and execution of e.g. an action listener only posted an event on the bus. Other components that needed to react on certain events registered themself as listeners for those events, and got hold of the data through that bus. The different views (MVC) didn't need to know about each other.

                    The drawback is that it makes it harder to follow the application flow if you just look at the code.

                    Kaj
                    • 7. Re: Eliminating a God Object (general how-to question)
                      796262
                      Thanks for the suggestions, everybody. I think, at least to start, I'm going to try splitting up the god class into a hierarchy of related Objects. So instead of having, for example, 10 codependent functions in the god class, I'll split those up into smaller classes that deal only with a subset of those functions. So if 5 of the functions deal with updating a JTable and the other 5 deal with a JList, I'll split them up into two classes: one for the JTable functions and one for the JList functions. I'll still have a central class for communication between these new Objects, but at least the responsibility will be a little more organized?
                      • 8. Re: Eliminating a God Object (general how-to question)
                        jschellSomeoneStoleMyAlias
                        jduprez wrote:
                        Just a quick note: pretentious as I might sound, I deem that God object is badly named: you don't care whether a single object holds a lot of the app's data, or whether most flows have to go through a single object. What has more impact on maintenability is when a single God Class(tm) holds too much behavior (business code).
                        In my case the God object was written in C++. It has more than 200 data members and more than 200 methods. It spanned at least three source files. (For those of you that know C++ that does not include the .h file.)

                        Pretty sure that is always going to be too much.
                        • 9. Re: Eliminating a God Object (general how-to question)
                          800387
                          [http://www.oodesign.com/single-responsibility-principle.html]

                          I guess one could have that object "play God" well, but that seems to violate the spirit of things.

                          - Saish
                          • 10. Re: Eliminating a God Object (general how-to question)
                            jschellSomeoneStoleMyAlias
                            jschell wrote:
                            jduprez wrote:
                            Just a quick note: pretentious as I might sound, I deem that God object is badly named: you don't care whether a single object holds a lot of the app's data, or whether most flows have to go through a single object. What has more impact on maintenability is when a single God Class(tm) holds too much behavior (business code).
                            In my case the God object was written in C++. It has more than 200 data members and more than 200 methods. It spanned at least three source files. (For those of you that know C++ that does not include the .h file.)

                            Pretty sure that is always going to be too much.
                            Forgot to mention the 20,000 lines of source code too.
                            • 11. Re: Eliminating a God Object (general how-to question)
                              jduprez
                              Just a quick note: pretentious as I might sound, I deem that God object is badly named: you don't care whether a single object holds a lot of the app's data, or whether most flows have to go through a single object. What has more impact on maintenability is when a single God Class(tm) holds too much behavior (business code).
                              In my case the God object was written in C++. It has more than 200 data members and more than 200 methods. It spanned at least three source files. (For those of you that know C++ that does not include the .h file.). Pretty sure that is always going to be too much.
                              I don't see the connection with (or against) my taxonomic remark: perdon my C++ ignorance, but does that mean your god object was composed of more than one class?

                              In support of my "god class" term, I found [this page|http://c2.com/cgi/wiki?GodClass] on the C2 wiki. Not necessarily authoritative, other C2 pages still refer to "GodObject", which I think is a much more widspread term, and which I'm sure I learnt there as well, althgough the corresponding page was hijacked since then into a (failed, IMHO) parody.

                              My point is, the Wikipedia page referred above is poor in content, and does not describe how a "god object" could be anything else than a "god class".
                              Please note how the C2 page also states "It is a form of the +MediatorPattern+, misapplied". I'm not sure whether it supports my point (see reply #1) that a Mediator or a tree of mediators is an acceptable way out, or if it deems that a mediator-who-knows-everyone is a bad design (probably true, but which a hierarchical tree of mediators is supposed to alleviate).

                              Edited by: jduprez on Mar 25, 2010 2:12 PM
                              • 12. Re: Eliminating a God Object (general how-to question)
                                jschellSomeoneStoleMyAlias
                                jduprez wrote:
                                perdon my C++ ignorance, but does that mean your god object was composed of more than one class?
                                One class only.