Did I properly encapsulate this package?

Question

My goal is to make File_Path_Handler constructor the only thing a user can access. Oh, is it too coupled? but the main question is: is File_Path_Handler the only accessible part of this package?

The handler:

package file_path_handler;
//| File_Path_Handler
//| - a component that lets the user pick a file, 
//| then the file path will be displayed accordingly
//| imports
 import javax.swing.JPanel; //? the base class
 import java.awt.BorderLayout; //? the organizing layout
public class File_Path_Handler extends JPanel {
 private File_Path_Load_Button button;
 private File_Path_Display display; //? the UI part
 private File_Path_Load_Button_Click_Listener trigger;
 private File_Path_Relay relay; //? the internal logic part
 //| constructor
 public File_Path_Handler( final String button_label, final String default_display_label ) {
 this.display = new File_Path_Display( default_display_label ); //? the UI part
 this.relay = new File_Path_Relay( display );
 this.trigger = new File_Path_Load_Button_Click_Listener( relay ); //? the logic that wires all the UI together
 this.button = new File_Path_Load_Button( button_label, trigger ); //? the other UI part
 this.setLayout( new BorderLayout() );
 this.add( button, BorderLayout.WEST );
 this.add( display, BorderLayout.CENTER );
 }
}

The display:

package file_path_handler;
// imports
 import javax.swing.JTextField; //? the base class
//| File_Path_Display
//| - displays the current selected file
class File_Path_Display extends JTextField { //? the public interface will be the JPanel that houses this couplet, this can stay hidden in the package
 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 ActionListener:

package file_path_handler;
//| import
 import java.awt.event.ActionListener;
 import java.awt.event.ActionEvent; //? the base interface
 import javax.swing.JFileChooser; //? for user input
//| File_Path_Load_Button_Click_Listener
//| - lets the user pick a file; then obtain_file_path_for( file )
class File_Path_Load_Button_Click_Listener implements ActionListener { //? package visibility: the JPanel that holds this all will be the public interface
 File_Path_Relay relay; //? the central control logic between UI input and output
 public void actionPerformed( ActionEvent action ) {
 JFileChooser file_chooser = new JFileChooser();
 int ret_val = file_chooser.showOpenDialog( null ); //? lets the user pick a file
 if ( JFileChooser.APPROVE_OPTION == ret_val ) { //? user succesfully picked a file
 relay.relay_file_path( file_chooser.getCurrentDirectory().toString() + file_chooser.getSelectedFile().getName() ); //? relay the path
 }
 }
 public File_Path_Load_Button_Click_Listener( File_Path_Relay relay ) { //? handed a relay by the housing
 this.relay = relay;
 }
}

The accompanying button:

package file_path_handler;
//| import
 import javax.swing.JButton; //? the base class
 import file_path_handler.File_Path_Load_Button_Click_Listener; //? the action that this button would perform
//| File_Path_Load_Button
//| - lets the user choose a file, then obtain_file_path_of(file)
class File_Path_Load_Button extends JButton { //? package visibility: this couplet scheme is the only one that will use File_Path_Load_Button
 private File_Path_Load_Button_Click_Listener trigger;
 public File_Path_Load_Button( final String button_label, File_Path_Load_Button_Click_Listener trigger ) { //? so it is given by the housing
 this.setText( button_label );
 this.addActionListener( trigger );
 }
}

The logical relay:

package file_path_handler;
//| imports
 import file_path_handler.File_Path_Display;
class File_Path_Relay { //? package visibility: the JPanel housing is the interface, this can stay hidden
 File_Path_Display display;
 public void relay_file_path( final String file_path ) {
 this.display.update_text( file_path );
 }
 public File_Path_Relay( File_Path_Display display ) {
 this.display = display;
 }
}

Answer 1

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
   

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

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

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

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

Answer 2

package file_path_handler;

Package names should be in the format tld.organization.application.package. For example like this:

package com.company.application.package;
package com.github.username.application;
package com.gmail.username.application;

This can be ignored for test applications and similar, but the moment you release code "into the wild" you should make sure that it is in a correct package.

---

//| File_Path_Handler
//| - a component that lets the user pick a file, 
//| then the file path will be displayed accordingly

---

What is this? You should always use JavaDoc to document your code. Not some obscure self-made format that is incompatible with everything.

If in doubt, look at the documentation of the language you're using. Never forget, you're not the first to use that language, you're not the first who needs whatever you need, somebody else already did it and they did it better in 99% of the cases.

---

//| imports
import javax.swing.JPanel; //? the base class
import java.awt.BorderLayout; //? the organizing layout

Again, these comments are not helpful in any way and should be left out.

---

public class File_Path_Handler extends JPanel {

This violates the Java Naming Conventions which state the classes should be UpperCamelCase.

---

private File_Path_Load_Button button;

This violates the Java Naming Conventions which state that the variables should be lowerCamelCase.

Also prefixing every variable with the class name is not helpful and not a good idea. It only clutters up everything and having a "common prefix" for variables is the whole idea of classes in the first place.

---

public File_Path_Handler( final String button_label, final String default_display_label ) {

This violates the Java Naming Conventions which states the functions should be lowerCamelCase with the exception of constructors which mimic the class name. Also variable names.

• user2738698: but how about its encapsulation?
• palacsint: @user2738698: Reviews don't have to be complete and may comment any part of the code (codereview.stackexchange.com/help/on-topic).
• user2738698: Uh huh, still want the encapsulation question answered though