Is it bad practice to have a class that holds pointers to most of the objects, and reference object through it?

I'm on my first bigger project, with quite a lot of component. And when I ran into the need to communicate between multiple objects of different classes, I decided to create a State class, that hold pointers to all the important object, and I pass it to every new object through the constructor. So 开发者_StackOverflow社区to access the object, I'll always go state.object.method(); I also use this to separate the server from the client when running at the same time, by having two state objects.

So, as I'm not an expert programmer (yet :) ), and I care a lot about the structure of thing program, and building it up in a "correct" manner, and I haven't seen this anywhere else. Is this a decent structure? Or is there some problems I don't see yet? Am I not thinking in the correct OO way?

If this is an OK way of doing it, I guess the hole question becomes subjective, and should not be the discussion here. If so, I'm sorry.

-Coder who worries about structure. (Who can sit up nights, trying to restructure, if he feels the code gets ugly)

You are better off using dependency injection where each class is passed all the objects it depends on. For notification/publication situations you can use a Observer pattern or listener pattern, where a component publishes data to a broker and the listeners listen to that data. Using these two model you should be able to cut your State down to less than 6 objects or remove it entirely.

You created a kind of manager object which handles the "state" of the application by managing a collection of objects.

A reference to this manager (which is close to a singleton in your application) is passed to the constructors of your application objects. This creates a sort of dependency injection with a twist, namely that the actual dependencies are retrieved via an extra indirection on usage.

I would say that whether or not this is a good design depends largely on your application model. Instead of public members I would use getter methods to retrieve the references giving you the option of on-demand creation.

If your applications' references are stable from the point of construction on (i.e. the objects that your manager keeps do not change over the application lifecycle) you could change your classes to use the manager reference passed on construction to retrieve the object references they need using them to initialise object reference attributes.

Much of the most optimal design depends on the model underlying your application. Things to look out for are overly static definition, creation of dependecies between objects, life cycle properties like on-demand creation of objects versus creation on application initialisation and the number of references between objects (if your manager references every other object in your application something is wrong.)

Based on the information you gave us I do not think your design is bad practice (improvements can always be made to any codebase, don't create your own moving target by keeping refactoring in an attempt to "perfect" your code.)