Reputation: 55
I have a dilemma in mind that I can't solve. Should I pass object in method parameter or in parent constructor in this case ?
First idea:
public class TempChannelsPlugin extends JolssyPlugin {
private JDA client;
private TempChannelsListener listener;
private JdaCommandManager jdaCommandManager;
@Inject
public TempChannelsPlugin(JDA client, JdaCommandManager jdaCommandManager, TempChannelsListener listener) {
super(Category.UTILITIES, "Temporary Channels");
this.client = client;
this.jdaCommandManager = jdaCommandManager;
this.listener = listener;
}
@Override
protected void setup() {
this.registerListener(this.client, this.listener);
}
}
And in JolssyPlugin:
protected void registerListener(JDA client, ListenerAdapter listenerAdapter) {
this.registerListeners(client, listenerAdapter);
}
Second idea:
public class TempChannelsPlugin extends JolssyPlugin {
private JDA client;
private TempChannelsListener listener;
private JdaCommandManager jdaCommandManager;
@Inject
public TempChannelsPlugin(JDA client, JdaCommandManager jdaCommandManager, TempChannelsListener listener) {
super(client, Category.UTILITIES, "Temporary Channels");
this.jdaCommandManager = jdaCommandManager;
this.listener = listener;
}
@Override
protected void setup() {
this.registerListener(this.listener);
}
}
And in JolssyPlugin:
protected void registerListener(ListenerAdapter listenerAdapter) {
this.registerListeners(this.client, listenerAdapter);
}
What is the difference between these two cases? And why should I favor one case over another?
Notice that I'm using Guice for DI
Upvotes: 2
Views: 145
Reputation: 38094
In my view, if method registerListener(ListenerAdapter listenerAdapter)
is located into TempChannelsPlugin
class:
protected void registerListener(ListenerAdapter listenerAdapter) {
this.registerListeners(this.client, listenerAdapter);
}
then the second option is better as vatiable private JDA client;
has a scope for the whole class. So there is no need to declare unnecessary parameter because we can use private JDA client;
variable in any place of class TempChannelsPlugin
.
In my view, if method registerListener(ListenerAdapter listenerAdapter)
is located to TempChannelsPlugin
class, then the second option is better as vatiable private JDA client;
has a scope for the whole class.
So there is no need to declare unnecessary parameter.
I would mode dependency of JDA client
to JolssyPlugin
as you have registerListener
in JolssyPlugin
.
By doing this we can remove one parameter from methods such as registerListener
and registerListeners
.
So code would look like this:
public abstract class JolssyPlugin {
protected Set<ListenerAdapter> listeners = new HashSet<>();
protected JdaCommandManager jdaCommandManager;
protected Category category;
protected String name;
private JDA jdaClient;
public JolssyPlugin(Category category, String name, JDA jdaClient) {
this.category = category;
this.name = name;
this.jdaClient = jdaClient;
}
protected abstract void setup();
protected void registerListener( ListenerAdapter listenerAdapter) {
this.registerListeners(client, listenerAdapter);
}
protected void registerListeners(ListenerAdapter... listenerAdapter) {
List<ListenerAdapter> listeners = Arrays.asList(listenerAdapter);
client.addEventListener(listeners);
this.listeners.addAll(listeners);
}
}
Upvotes: 2