6
\$\begingroup\$

I've created a Browser class that's working as it should, with 4 JButtons.

What I'm working on now is understanding object-oriented programming a bit more (mostly been working with Python), so I thought it would be good practice to try and split up my Browser class into 2-3 classes.

For example, I would like someone to be able to change the JPanel and JFrame without having to look through methods like newURL that has nothing to do with the GUI.

I've tried splitting it up, but I just don't understand OOP well enough to make the code work with more than one class. Any ideas on where I can read up on this, or maybe you could give me a few tips on what I can/can't move around in the code?

import java.awt.BorderLayout;
import java.awt.event.*;
import java.util.ArrayList;
import javax.swing.*;
import javax.swing.event.*;

@SuppressWarnings("serial")
public class Browser extends JFrame {

private JTextField addressBar;
private JEditorPane display;
private JButton button;
private JPanel panel;
private ArrayList<String> list = new ArrayList<>();
private ArrayList<String> history = new ArrayList<>();
private String URL = new String();
private int i = -1;
private int j = -1;
private JButton backward;
private JButton forward;
private JFrame frame;

public Browser() {
    frame = new JFrame("My Browser");
    panel = new JPanel();
    addressBar = new JTextField("Enter a URL");
    addressBar.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            URL = e.getActionCommand().toString();
            newURL(URL);
        }
    });
    display = new JEditorPane();
    display.setEditable(false);
    display.addHyperlinkListener(new HyperlinkListener() {
        public void hyperlinkUpdate(HyperlinkEvent e) {
            if (e.getEventType() == HyperlinkEvent.EventType.ACTIVATED) {
                URL = e.getURL().toString();
                newURL(URL);
            }
        }
    });
    addButton("Close").addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            if (e.getSource() != null) {
                closeAction();
            }
        }
    });

    forward = addButton("Forward");
    forward.setEnabled(false);
    forward.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            if (e.getSource() != null) {
                forwardAction();
            }
        }
    });

    backward = addButton("Back");
    backward.setEnabled(false);
    backward.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            if (e.getSource() != null) {
                backwardAction();
            }
        }
    });

    addButton("History").addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            if (e.getSource() != null) {
                historyAction();
            }
        }
    });
    setframe();
}

private void setframe() {
    frame.add(addressBar, BorderLayout.NORTH);
    frame.add(panel, BorderLayout.SOUTH);
    frame.add(new JScrollPane(display), BorderLayout.CENTER);
    frame.setSize(400, 400);
    frame.setVisible(true);
}

private JButton addButton(String name) {
    button = new JButton(name);
    panel.add(button);
    return button;
}

private void setBackward(JButton button) {
    if (i > 0) {
        button.setEnabled(true);
    } else {
        button.setEnabled(false);
    }
}

private void setForward(JButton button) {
    if (i < list.size() - 1) {
        button.setEnabled(true);
    } else {
        button.setEnabled(false);
    }
}

private void loadURL(String URL) {
    try {
        display.setPage(URL);
        addressBar.setText(URL);
    } catch (Exception e) {
        list.remove(list.size() - 1);
        history.remove(history.size()-1);
        JOptionPane.showMessageDialog(null, "fel länk");
    }
}

private void checklist() {
    if (i != j) {
        list.subList(i, list.size()).clear();
        j = i;
    }
}

private void newURL(String URL) {
    i++;
    j++;
    checklist();
    loadURL(URL);
    history.add(URL);
    list.add(URL);
    setBackward(backward);
    setForward(forward);
}

private void backwardAction() {
    i--;
    setForward(forward);
    setBackward(backward);
    loadURL(list.get(i));
}

private void forwardAction() {
    i++;
    setBackward(backward);
    setForward(forward);
    loadURL(list.get(i));
}

private void closeAction() {
    System.exit(0);
}

private void historyAction() {
    String html = new String();
    for (String link : history) {
        html = html + "<a href=\"" + link + "\">" + link + "</a>\n";
    }
    html = "<html><body" + html + "</body></html>";
    JEditorPane ep = new JEditorPane("text/html", html);
    ep.addHyperlinkListener(new HyperlinkListener() {
        public void hyperlinkUpdate(HyperlinkEvent e) {
            if (e.getEventType().equals(HyperlinkEvent.EventType.ACTIVATED)) {
                loadURL(e.getURL().toString());
            }
        }
    });
    ep.setEditable(false);
    JOptionPane.showMessageDialog(null, ep);
}
    public static void main(String[] args) {
    Browser chrome = new Browser();
    chrome.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
}

}

Some things to know if you've copied the code: The functionality of the buttons are in the methods jbuttonAction. setJButton helps decide when the forward and backwards buttons should be activated, the JButton history should have clickable links and if you want to visit a website, write: https://www.website.com.

I've thought about maybe splitting it up like this: 1 class with: setframe() and addbutton(). 1 class with: buttonAction() (all 4 methods), setButton() (both methods) and 1 class with: main, newURL(), loadURL().

Any help on how this structure could be implemented in my code would be greatly appreciated. Should the constructor still look the same? Which class should extend which?

EDIT: Follow-up: Follow up: utilizing OOP, is the use of static OK here?

\$\endgroup\$
  • \$\begingroup\$ Yes, although it could use a bit more context. Why are you building it, what features does it currently have, etc. However, cross-posts should be declared and you hadn't done so, so I did. Removing it at Stack Overflow is exactly the right step. \$\endgroup\$ – Mast Dec 10 '17 at 14:08
  • 1
    \$\begingroup\$ You should show the import-statements in the code. This code cant be compiled because their are missing. \$\endgroup\$ – Dexter Thorn Dec 10 '17 at 14:14
  • \$\begingroup\$ @Mast See edited text \$\endgroup\$ – armara Dec 10 '17 at 22:20
  • \$\begingroup\$ @DexterThorn See edited code \$\endgroup\$ – armara Dec 10 '17 at 22:21
  • \$\begingroup\$ Follow-up question \$\endgroup\$ – 200_success Feb 6 '18 at 13:09
2
\$\begingroup\$

Oh boy where to start. You could use a full course on OOP which is out of scope for an answer here. So I'll attempt to give you the required starting points from which you should be able to figure it out yourself instead. Things may still be unclear but I'll try to use the common terms so that you can start recognising them when you search online for more information. (Tip: The book "head first design patters" can be a great help at the stage you're currently at).

First things first:

Which class should extend which?

For this little program you most likely don't need any inheritance at all. You should even remove the extends JFrame from the only class you have at this point. Everything should still work the same.
A class only inherits another class if you can say "Myclass IS this other class". That other class can be an actual class, an abstract class or an interface. The idea is the same. For example: An ArrayList is a List. This means you can (and should!) say:

 List<> history = new ArrayList<String>;

getX() and setX(someX) are reserved to modify the fields of a class. The expected implementations usually look like this:

public class Example {
    private String name;

    public String getName() {
        return name;
    }

    public void setName(String name) {
        this.name = name;
    }
}

Pro tip: only provide a setter if it makes sense to actually modify that field from outside. Usually you want to set that field exactly once in the constructor and from then on never change it again.

Your current setBackward(JButton button) and forward methods should be renamed to something like initBackward() or updateBackward(). Notice that I also removed the button passed into that method? Inside this class you can already access the private button, so you can directly update the backward and forward buttons state.


Now for the major change.

For example, I would like someone to be able to change the JPanel and JFrame without having to look through methods like newURL that has nothing to do with the GUI.

Awesome! You realise it would be a good idea to split up GUI from "business logic" already. That's a great start actually. Only your lack of experience means you get stuck here. For me, this screams MVC! But what is that thing?

MVC stands for Model-View-Controler. It separates your program into some data representation, a GUI and something that binds the 2 things.

In your case, the View would be the Browser class. It holds all buttons, panel stuff, handles button clicks and key presses. Only that handling in this case means nothing more than telling the Controller something like: handle "forward" action.

The Model is most likely some way to represent the actual history. It will contain a list of previously visited pages and probably some way to tell which is the currently shown page.

The controller binds the 2. It translates a message from the View (handle "forward" action) into something the model understands (String newURL = model.getNext()) and probably also provides some feedback to the view (browser.updateCurrentPage(newURL)).

As you can see we will have 3 classes here. Browser which is basicaly a stripped down version of your current class. History which will contain the logic and data behind a browser history. And Controller to bind the other two together.

Let's start with how to create the 3 classes and bind them together so they can actually communicate. First the Browser class, which now also receives a reference to the controller in it's constructor:

public class Browser {
    private Controller controller;
    //... rest of fields

    public Browser (Controller controller) {
        this.controller = controller;
        //...
    }

I'm going to skip the History for a moment since it's exactly the same as the Browser class and just jump right into the Controller.

public class Controller {
    private Browser browser;
    private History history;

    public Controller() {
        browser = new Browser(this);
        history = new History(this);
    }

And that's all there is to it to bind them all. (Note there are other options where you don't need to be able to reference back. For example, the History probably doesn't need to know about any controller classes since it will always be the controller initiating the calls to the History instance anyway.

In that case the field initialisation in the controller class becomes history = new History(); and the History class doesn't take any parameters during construction.


With the construction and linking out of the way, let's take a look in how to actually make them work together. Take the back button for example. As mentioned before, the Browser just tells the Controller to go back:

public class Browser {
    private Controller controller;
    private JButton backButton;
    //... rest of fields

    public Browser (Controller controller) {
        this.controller = controller;
        //...
        backward = addButton("Back");
        backward.setEnabled(false);
        backward.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e) {
                if (e.getSource() != null) {
                    controller.goBack();
                }
            }
        });
    }

    public void updateCurrentPage(String newPage){
        // actually navigate to that page here
    }

As for the Controller, it will do 2 things. Fetch the previous page from the History and update the Browser with this new information:

public void goBack() {
    String newUrl = history.goBack();
    browser.updateCurrentPage(newUrl);
}

And finally, the History provides a way for the controller to actually retrieve that previous page and at the same time updates it's internal tracker of which page is active at the moment.

public class History {
    private List<> previousPages = new ArrayList<String>();
    private int currentPageIndex = -1;

    public String goBack(){
        if(currentPageIndex < 0){
            // default page? throw error? your choice how to handle this
        }
        String result = previousPages.get(currentPageIndex);
        currentPageIndex--;
        return result;
    }

We may have to add a bit of logic to know if the previous and next buttons should be enabled. Let's look at the controller method for goBack to see how this could work:

public void goBack() {
    String newUrl = history.goBack();
    browser.updateCurrentPage(newUrl);
    browser.setBackEnabled(history.hasPrevious());
    browser.setNextEnabled(true); //depends on if history can return a default page perhaps?
}

I've left some notes on design decissions you'll have to make yourself. This answer is already getting rather long so I'm not going to go into detail about those.


I hope this gives you a decent start to split up your program. If you get it to work it would be a great idea to post a follow up question here too. And feel free to ping me if you do so I can see what you made of it. At that point we can give a more detailed review on the conventions and minor possible improvements as well.

\$\endgroup\$
  • \$\begingroup\$ Thank you so much for this! I took most of your tips and created the three classes you mention. I think I got it to work now and it looks much better than before. Where should I post the follow up question along with my new code? Should I press "Answer your question", or should I just edit my initial post? \$\endgroup\$ – armara Feb 3 '18 at 13:01
  • \$\begingroup\$ Never edit the initial post with new code as it would make this answer obsolete! Just create a new post on this site and add a link to this question. If you want you can also edit this question here to link to the new follow up to let people know it's no longer useful to add answers to this one. \$\endgroup\$ – Imus Feb 5 '18 at 8:51
  • \$\begingroup\$ Done! Added the new link to my initial question above. \$\endgroup\$ – armara Feb 6 '18 at 12:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.