- A great comment from @tb-'s answer @tb-'s answer:
See also: [#1 at How many squares can you see?][sq] [sq]: httphttps://codereview.stackexchange.com/a/45683/7076
- A great comment from @tb-'s answer:
See also: [#1 at How many squares can you see?][sq] [sq]: http://codereview.stackexchange.com/a/45683/7076
- A great comment from @tb-'s answer:
See also: [#1 at How many squares can you see?][sq] [sq]: https://codereview.stackexchange.com/a/45683/7076
Oh, is it too coupled?
It looks OK for me (but apply the suggestions by Bobby and above).
is
File_Path_Handler
the only accessible part of this package?
It seems to me, that it is.
Another notes:
- You cuold use
fileChooser.getSelectedFile().getAbsolutePath()
here:
relay.relay_file_path( file_chooser.getCurrentDirectory().toString() + file_chooser.getSelectedFile().getName() ); //? relay the path
- A great comment from @tb-'s answer:
Do not extend
JPanel
, instead have a privateJPanel
attribute and deal with it (Encapsulation). There is no need to give access to allJPanel
methods for code dealing with aUserPanel
instance. If you extend, you are forced to stay with this forever, if you encapsulate, you can change whenever you want without taking care of something outside the class.
See also: [#1 at How many squares can you see?][sq] [sq]: http://codereview.stackexchange.com/a/45683/7076
- I'd use a factory method instead of inheritance:
class File_Path_Display extends JTextField { public File_Path_Display( final String default_label ) { this.setText( default_label ); this.setEnabled( false ); } public void update_text( final String file_path ) { this.setText( file_path ); } }
The following is much simpler:
private JTextField createFilePathDisplay(String defaultDisplayLabel) {
JTextField filePathDisplay = new JTextField();
filePathDisplay.setText(defaultDisplayLabel);
filePathDisplay.setEnabled(false);
return filePathDisplay;
}
You can call setText
directly (it's inherited), I don't see any additional value of update_text
.
You could create a similar method instead of File_Path_Load_Button
too.
(If you really want to do it for information hiding you should use composition instead of inheritance. Related: Effective Java, Second Edition, Item 16: Favor composition over inheritance.)
According to the Java Code Conventions constructors should be the first in a class, before the other methods.
The
trigger
field is unused here:
class File_Path_Load_Button extends JButton { private File_Path_Load_Button_Click_Listener trigger; public File_Path_Load_Button( final String button_label, File_Path_Load_Button_Click_Listener trigger ) { this.setText( button_label ); this.addActionListener( trigger ); } }
You could remove it. (Although the factory method is still a better idea.)