Small notepad editor

Question

I decided on making a small notepad editor in Java using Swing and I think it functions pretty well. Unfortunately, my skills in making my project tidy and organized has abandoned me; it looks like spaghetti code, which many renowned programmers discourage from doing.

What can I do to make my code simpler?

Note: The JFontChooser.java does not belong to me. The entire file belongs to ZoeloeSoft.

Project

FramePanel.java - Adds all editable JTextArea

// events and actions
import java.awt.*;
import java.awt.event.*;
import javax.swing.JCheckBoxMenuItem;
import java.io.File;

// for gui-related
import javax.swing.*;

// for opening/saving files
import javax.swing.filechooser.*;

public class FramePanel implements ActionListener
{
    private Frame window;
    private JPanel panel;
    private JTextArea edit;
    private File openFile;
    private JScrollPane scroll;
    private JFileChooser dialog;
    private Utilities util;

    // instantiate the panel
    public FramePanel(Frame caller)
    {
        // instantiate the utilites class and filechooser for function
        util = new Utilities();
        dialog = new JFileChooser();
        openFile = null;

        // instantiate the panel with borderlayout and frame
        panel = new JPanel();
        window = caller;
        panel.setLayout(new BorderLayout());

        // instantiate the scrollpanel and editarea
        edit = new JTextArea();
        scroll = new JScrollPane(edit);

        // customize editarea
        edit.setFont(new Font("Courier New", Font.PLAIN, 12));
        edit.setTabSize(4);

        // add the scrollpane with editarea in the panel
        panel.add(scroll, BorderLayout.CENTER);
    }

    // pass on panel to frame
    public JPanel getPanel() { return panel; }

    // listen for actions/clicks
    public void actionPerformed(ActionEvent e)
    {
        String query = e.getActionCommand();

        // if new was clicked > erase document to null
        if (query.equals("New")) {
            edit.setText(null);
            window.setName(null);
            openFile = null;
        }

        // if open was clicked > open new file
        else if (query.equals("Open"))
        {
            int result = dialog.showOpenDialog(dialog);

            if (result == JFileChooser.APPROVE_OPTION) {
                String fileName = util.openFile(edit, dialog.getSelectedFile());
                openFile = dialog.getSelectedFile();

                window.setName(fileName);
            }
        }
        // if save was clicked > save if new file; overwrite if existing
        else if (query.equals("Save"))
        {
            if (openFile != null && openFile.exists())
                util.saveFile(edit, openFile, true);
            else
            {
                int ret = dialog.showSaveDialog(dialog);

                if (ret == JFileChooser.APPROVE_OPTION) {
                    String name = util.saveFile(edit, dialog.getSelectedFile(), false);
                    openFile = dialog.getSelectedFile();
                }

                window.setName(openFile.getName());
            }
        }
        // if exit was clicked > exit
        else if (query.equals("Exit"))
            window.exit();
        // if select all was clicked > select all text
        else if (query.equals("Select All"))
            edit.selectAll();
        // if time/date was clicked > append time and date.
        else if (query.equals("Time/Date"))
            edit.append(util.getDate());
        // if word wrap was selected > set editable to word wrap t/f
        else if (query.equals("Word Wrap"))
        {
            JCheckBoxMenuItem opt = (JCheckBoxMenuItem)e.getSource();
            edit.setLineWrap(opt.isSelected());
        }
        // if font was selected > set editable.font to selected font
        else if (query.equals("Font"))
            util.changeFont(edit);
    }
}

Utilities - Adds functionality to FramePanel.java

// for editable
import javax.swing.JTextArea;
import java.awt.Font;

// for opening a file
import java.io.File;
import java.io.FileReader;
import java.util.Scanner;
import javax.swing.JCheckBoxMenuItem;

// for writing a file
import java.io.BufferedWriter;
import java.io.FileWriter;
import java.io.IOException;

// for appending dates
import java.util.Date;
import java.text.DateFormat;
import java.text.SimpleDateFormat;

public class Utilities
{
    // open file; should only be called from framepanel and requires
    // jfilechooser and editable
    public String openFile(JTextArea editable, File file)
    {
        editable.setText(null);

        // instantiate the scanner and make string pointing to filepath
        Scanner fileReader = null;
        String filePath = file.getPath();

        // try to open file
        try
        {
            fileReader = new Scanner(new FileReader(filePath));

            // loop until all files are appended to editable
            while (fileReader.hasNextLine())
                editable.append(fileReader.nextLine() + "\n");
        }
        // catch any errors and exceptions
        catch (Exception except) {
            except.printStackTrace();
        }
        // close the reader
        finally {
            fileReader.close();
        }

        return file.getName();
    }

    // save file; should only be called from framepanel and requires
    // jfilechooser and editable
    public String saveFile(JTextArea editable, File file, boolean overWrite)
    {
        String fileName = file.getName();
        String text = editable.getText();
        BufferedWriter writer = null;

        if (overWrite)
        {
            try
            {
                writer = new BufferedWriter(new FileWriter(file.getAbsolutePath()));
                writer.write(""); // to clear out text
                writer.append(text);
            }
            catch (Exception e) { e.printStackTrace(); }
            finally
            {
                try
                {
                    writer.flush();
                    writer.close();
                }
                catch (Exception e) { e.printStackTrace(); }
            }

            fileName += "0";
        }
        else
        {
            try
            {
                writer = new BufferedWriter(new FileWriter(file.getAbsolutePath()));
                writer.write(text);
            }
            catch (Exception e) { e.printStackTrace(); }
            finally
            {
                try
                {
                    writer.flush();
                    writer.close();
                }
                catch (Exception e) { e.printStackTrace(); }
            }

            fileName += "1";
        }

        return fileName;
    }

    // gives date to caller
    public String getDate()
    {
        DateFormat dateList = new SimpleDateFormat("hh:mm MM/dd/yyyy");
        Date date = new Date();
        return dateList.format(date);
    }

    // changes font of Jtextarea
    public void changeFont(JTextArea editable)
    {
        Font current = editable.getFont();
        JFontChooser dialog = new JFontChooser(null);
        dialog.setFont(current);
        int ret = dialog.showDialog();

        if (ret == dialog.OK_OPTION)
            editable.setFont(dialog.getFont());
    }
}

Answer 1

General

• Java puts the { opening brace on the same line as the control-block, not the next line:

      if (overWrite)
      {
          try
          {
              ....
  

  should be:

      if (overWrite) {
          try {
              ....
  

• Always use braces even for simple '1-liner' blocks (this is what apple did not do recently and caused massive problems when a but was introduced.... with a small change):

          while (fileReader.hasNextLine())
              editable.append(fileReader.nextLine() + "\n");
  

  should be:

          while (fileReader.hasNextLine()) {
              editable.append(fileReader.nextLine() + "\n");
          }
  

• FileReader can take a File as input. You don't need to convert the File, to a Path, and to a String for FileReader.

• when reading a bunch of text from a file, and dumping it unchanged to an editable panel, there is no need to read it line at a time... you can read it in a bulk format which is faster.

Utilities

You have done things really well, in one sense, your use of try-catch is extensive, and you are closing streams in the appropriate places ... but, (this one will make you cringe)... have you read up on the try-with-resources functionality in java7? Actually, in this particular case, you will likely be well served with the new-in-Java7 Files class (Javadoc)

As far as I can tell, the overwrite flag makes no difference in the saveFile....

I tend to force a charset for all files. This may not work for you, in which case you can remove the StandardCharsets.UTF_8 arguments, and rely on the platform default, but this may mean a file you save on one computer creates garbage on another.

Rewriting your Utilities class:

public String openFile(JTextArea editable, File file) {
    try {
        editable.setText(new String(Files.readAllBytes(file.toPath()), StandardCharsets.UTF_8));
    } catch (IOException e) {
        e.printStackTrace();
    }
    return file.getName();
}

// save file; should only be called from framepanel and requires
// jfilechooser and editable
public String saveFile(JTextArea editable, File file, boolean overWrite) {

    String fileName = file.getName();

    try {
        Files.write(file.toPath(), editable.getText().getBytes(StandardCharsets.UTF_8));
        if (overWrite) {
            fileName += "0";
        } else {
            fileName += "1";
        }

    } catch (IOException e) {
        e.printStackTrace();
    }
    return fileName;
}

FramePanel

Going to leave this class for someone who knows Swing better than I do.

Answer 2

FramePanel

• Use local variables instead of instance variables if they're only used in one method.

• I wouldn't implement ActionListener but instead add each action with a new ActionListener, either as an anonymous inner class, or if they get complex, a separate class.

• Look at MVC examples like GUI Classes and Listeners to see ways of separating this functionality.

Error Handling

There are a few things to keep in mind when adding error handling to a program:

1. Log debugging information for the developer.

   Consider using a logger instead of printStackTrace(), and log extra information on what the program was doing along with the exception. This can be sent to a file which can be accessed afterwards. Error output will usually be lost when a user runs a Swing program because they're not running in a console.

2. Clean up resources.

   try
   {
       writer = new BufferedWriter(new FileWriter(file.getAbsolutePath()));
       writer.write(""); // to clear out text
       writer.append(text);
   }
   catch (Exception e) { e.printStackTrace(); }
   finally
   {
       try
       {
           writer.flush();
           writer.close();
       }
       catch (Exception e) { e.printStackTrace(); }
   }
   

   Closing the writer in the finally block is good, but you should check for null because the writer might not be set after an exception has occurred. The finally block is repetitive enough to put in a util method. Try-with-resource or alternative library methods take care of a lot of this boilerplate.

3. Display feedback for the user.

   If there's an error saving the file, no feedback is displayed to the user. The user only finds out later when they try to open the file, after they've lost their work.

4. Resume execution from a consistent state.

   String fileName = util.openFile(edit, dialog.getSelectedFile());
   openFile = dialog.getSelectedFile();
   
   window.setName(fileName);
   

   After an error, the previous file contents are still displayed, but the open file has changed to the new one. This is an inconsistent state, and accidently saving here can overwrite the contents of the new file with the old one.

   So the util methods can't deal with the exception by themselves because the ActionListeners have their own error handling to do. The util methods should either return a error status, or throw the exception to let the caller know there's been an error.

Answer 3

FramePanel

In java 7, you can use a String in a switch statement. You can use this to decide what do to in the actionPerformed. I would also split that method up to make the overall code more readable:

switch(query){
    case "New":
        eraseDocument();
        break;
    case "Open":
        openDocument();
        break;
   //...
}