Open HelgeStenstrom opened 5 years ago
Ouh i have to see the code to answer you, actually i think you can create a Polyspiral without creating an Oscilloscope, can you send me the code that is proving the opposite :)?
Look at the constructors for these classes:
public Polyspiral(final VisualizerDrawer visualizerDrawer) {
this.visualizerDrawer = visualizerDrawer;
}
public Oscilloscope(VisualizerDrawer visualizerDrawer) {
this.visualizerDrawer = visualizerDrawer;
}
and the beginning of VisaulizerDrawer:
public class VisualizerDrawer extends VisualizerModel {
private final Oscilloscope oscilloscope = new Oscilloscope(this);
private Polyspiral polySpiral = new Polyspiral(this);
protected final Sierpinski sierpinski = new Sierpinski(this);
private final JuliaSet juliaSet = new JuliaSet(this);
private Sprites3D sprite3D = new Sprites3D(this, Shape3D.SPHERE);
When you create Oscilloscope or PolySpiral, you need to provide a VisualizerDrawer as argument. When you create a VisualizerDrawer, both Oscilloscope and PolySpiral will be created.
Aouuuu now i catch you, i was Junior developer and didn't knew well refactoring concepts back then. I see... your point.
@HelgeStenstrom Please run the application after refactorings because it ,may break , i am trying to run it after the merge requests and now i am getting full of errors , i will fix but be careful :
I didn’t run it. Your problem above looks like a compile time problem, and I doubt that it’s caused by my changes. At least not by one particular change, but perhaps if merge conflicts were not properly solved.
It was due to making
I have made a branch where Oscilloscope and its siblings can now be instantiated independently of each other. They are no longer owned by VisusalizerDrawer, but by a new class GadgetOwner.
This might be a step towards a better structure, but there are still issues. The instances are now created upon initialization of PaintService. (The GadgetOwner object is created there and then, with its owned objects). It would be better to inject the GadgetOwner into the PaintService object.
GadgetOwner is not a good name, but it will do for now.
Of the five objects owned by GadgetOwner, I have tested four. I didn't manage to visualize using the JuliaSet from the GUI, but I think it would work.
Take a look at https://github.com/HelgeStenstrom/XR3Player/tree/refactorOscilloscope
I can make a pull request from it, if you want, but it's not ready for merging.
I fooled myself: my refactoring did not remove the cross-dependence between VisualizerDrawer and the "gadgets". I just moved the point where these items are created. I have to think a bit more.
Again I have to correct myself. The refactoring did help. That is because the creation of Oscilloscope and siblings are done at creation of Visualizer, but Oscilloscope only needs a reference to an VisualizerDrawer. So in a unit test, I can create a VisualizerDrawer, and pass it to a new Oscilloscope. No Visualizer will be created, and therefore no siblings of Oscilloscope. Right now the creation takes place in the constructor of PaintService, but that doesn't feel like a natural place. Better to have it in Visualizer. In practice, there is not much difference.
@HelgeStenstrom Give it a pull request i like your idea, we will make a library from iy and completely remove it from XR3Player so we will use the library :)
I will will finish it for you, let me see how it looks like :)
I want to make XR3Player professional product like Traktor and VirtualDJ, it's not long from there and in some places it has more features.
Imagine it having 20. 000 stars one day and you are a contributor :)
If I understand correctly, once the player is started, we have 5 instances of Oscilloscope, 5 instances of JuliaSet, and so on. How many of these may at most be visible at the same time? I don’t know the lifetime of these objects. Do they live until the application is closed? For the object called player0 (if I remember correctly), is there a need to show both Oscilloscope and Sierpinsky or JuliaSet simultaneously? If not, they can be created when they need to be shown, and left to garbage collection when they don’t. On the other hand, I don’t think they take much resources. But I care about understandable code.
We have 3 XR3PlayerControllers (each one representing a player), each one containing a Visualizer which has 1 instance of all the above mentioned before .
This below is an XR3PlayerController class :
You can see 3 of them in the application . Inside it contains the visualizer the djdisc etc etc etc all the objects .
It seems to me that it's currently not possible to create an instance of Oscilloscope, without also creating an instance of Polyspiral, and vice versa. I think that's unfortunate. Am I right, and can that dependency be broken?
Apart from these two classes, the same dependency is between instances of all classes created at lin 24-28 of VisualizerDrawer.