Skip to main content
Code Review

Return to Answer

replaced http://codereview.stackexchange.com/ with https://codereview.stackexchange.com/
Source Link
  1. 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

  1. 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

  1. 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

Source Link
palacsint
  • 30.3k
  • 9
  • 82
  • 157

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:

  1. You cuold use fileChooser.getSelectedFile().getAbsolutePath() here:
relay.relay_file_path( file_chooser.getCurrentDirectory().toString() + 
 file_chooser.getSelectedFile().getName() ); //? relay the path
  1. A great comment from @tb-'s answer:

Do not extend JPanel, instead have a private JPanel attribute and deal with it (Encapsulation). There is no need to give access to all JPanel methods for code dealing with a UserPanel 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

  1. 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.)

  1. According to the Java Code Conventions constructors should be the first in a class, before the other methods.

  2. 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.)

lang-java

AltStyle によって変換されたページ (->オリジナル) /