I'm designing a simple GUI using tabs to separate each 'window' of the application. There are 3 main tabs, each with 2 sub-tabs.
In one class I create the entire hierarchy of panes and panels:
mainPane = new JTabbedPane();
mainPane.setBounds(0, 0, WIDTH, HEIGHT);
//instantiate main tabs, get panes added to them
createTab = new JPanel();
createTab.setLayout(null);
customizeTab = new JPanel();
customizeTab.setLayout(null);
viewTab = new JPanel();
viewTab.setLayout(null);
//instantiate main panes, get sub-tabs added to them
createPane = new JTabbedPane();
createPane.setBounds(0, 0, WIDTH, HEIGHT);
customizePane = new JTabbedPane();
customizePane.setBounds(0, 0, WIDTH, HEIGHT);
viewPane = new JTabbedPane();
viewPane.setBounds(0, 0, WIDTH, HEIGHT);
//instantiate sub-tabs, get elements added to them
createEntityTab = new JPanel();
createEntityTab.setLayout(null);
createDealTab = new JPanel();
createDealTab.setLayout(null);
customizeEntityTab = new JPanel();
customizeEntityTab.setLayout(null);
customizeDealTab = new JPanel();
customizeDealTab.setLayout(null);
viewEntityTab = new JPanel();
viewEntityTab.setLayout(null);
viewDealTab = new JPanel();
viewDealTab.setLayout(null);
//add sub-tabs to main panes
createPane.addTab("Create Entity",null, createEntityTab);
createPane.addTab("Create Deal", null, createDealTab);
customizePane.addTab("Customize Entity", null, customizeEntityTab);
customizePane.addTab("Customize Deal", null, customizeDealTab);
viewPane.addTab("View Entity", null, viewEntityTab);
viewPane.addTab("View Deal", null, viewDealTab);
//add main panes to main tabs
createTab.add(createPane);
customizeTab.add(customizePane);
viewTab.add(viewPane);
//add main tabs to mane pane
mainPane.addTab("Create", null, createTab, "");
mainPane.addTab("Customize", null, customizeTab, "");
mainPane.addTab("View", null, viewTab, "");
and now to add elements to each sub-tab, I would like to do something like this:
new CreateEntity(createEntityTab);
new CreateDeal(createDealTab);
i.e., passing the "sub-tab" and then adding elements in the CreateEntity
class.
This seems to work, but my question is the following:
Is using a constructor to pass a JPanel, and then adding elements in the new class considered bad coding?
I know using null
in layout is bad practice. It's just for the question.
2 Answers 2
Is using a constructor to pass a JPanel, and then adding elements in the new class considered bad coding?
Not necessarily. However, if you only create an instance of the other class just to do things in the constructor, then you don't need the new class - and in that case it is considered bad coding.
If that's all you do in the constructor, then move the code from the constructor to a method (which you can place more or less anywhere, but I'd recommend having it in the original class unless you feel that the original class gets too big).
private void createEntityTab(JPanel panel) {
...
panel.add(...);
...
}
Actually, it might be even better to let this method return the JPanel
, so that you can use this code:
createPane.addTab("Create Entity", null, createEntityTab());
And use the method:
private JPanel createEntityTab() {
JPanel panel = new JPanel();
panel.setLayout(null);
...
panel.add(...);
...
return panel;
}
-
1\$\begingroup\$ I agree that, if all you are doing in the class is in the constructor, then you don't need/want another class, and creating one just for this purpose wouldn't be a best coding practice. Create a class to represent one or more 'things' (concepts, structures, entities, whatever) in your program, not just to hold methods or constructors. \$\endgroup\$rcook– rcook2014年05月25日 15:16:04 +00:00Commented May 25, 2014 at 15:16
-
\$\begingroup\$ @rcook Thank you, that's my point exactly! If the class is not more than a constructor with code, then it should be a method. \$\endgroup\$Simon Forsberg– Simon Forsberg2014年05月25日 15:35:35 +00:00Commented May 25, 2014 at 15:35
-
\$\begingroup\$ Thanks for your answer! My intention is that the CreateEntity class will contain all the elements (buttons, textfields...) and corresponding action listeners for that tab. This is just to avoid having all the GUI code in one class, which will easily mount up to over 600 lines of code. No matter how tidy I am, I don't like scrolling! Thanks again! \$\endgroup\$Jona– Jona2014年05月25日 19:51:04 +00:00Commented May 25, 2014 at 19:51
I don't see anything wrong with it. You are creating an object that is going to use that data, at construction time, to set itself up. I don't see anything objectionable if that data happens to be a UI Panel. I would think it would be (very slightly) worse to force a programmer to first use the constructor and then use a setter method to do the same thing; conceptually this links the construction with the panel, and it sounds like that fits the model you are making.
-
\$\begingroup\$ Welcome to Code Review! I think that technically your answer is correct, but I don't think that is what the OP is doing here. We have understood the question in two different ways, and I think that for each way, both of us is right. As I understand, the
CreateEntity
classextends Object
and only contains a constructor (no fields, no methods, no nothing!) \$\endgroup\$Simon Forsberg– Simon Forsberg2014年05月25日 15:07:12 +00:00Commented May 25, 2014 at 15:07
CreateEntity
class. \$\endgroup\$