2
\$\begingroup\$

I am not very familiar with Swing but am attempting to make a Reddit GUI. I've finally got everything to work as expected but I must admit, it's quite mundane and not very nice to look at. I want to add some visually appealing things to it but as I said I am very new to Swing. I got help making the components and such for my idea, so I'm feeling a little over-whelmed.

What the program currently does is fetches headlines from the top of subreddits that the user inputs. As you can see from the screenshot, it isn't nice to look at as I was saying. Any advice design wise would be appreciated!

Screenshot of program

P.S. This is definitely advanced for me, so just want to side note I didn't somehow create this from scratch without help. Also have since fixed the crooked Reddit icon.

package com.Will.me;
 
import org.jsoup.Jsoup;
import org.jsoup.nodes.Document;
import org.jsoup.select.Elements;
 
import javax.swing.*;
import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
 
public class Reddit extends JFrame {
 
 
    static JPanel mainPanel = new JPanel();
    static JTextField textField = new JTextField(20);
    static JButton search = new JButton("Search");
    static JButton[] numbers = new JButton[28];
    static JButton[] headlines = new JButton[28];
    static JLabel[] votes = new JLabel[28];
    static JLabel title = new JLabel("\tHeadlines from r/");
 
    public Reddit(){
        mainPanel.setLayout(null);
        setUpComponents();
 
        title.setVisible(false);
 
        search.addActionListener(new ActionListener() {
            @Override
            public void actionPerformed(ActionEvent e) {
 
                title.setVisible(true);
                String str = textField.getText();
                title.setText("");
                title.setText("\tHeadline from r/"+str);
 
                getSubReddit(str);
 
 
            }
        });
 
 
        add(mainPanel);
        setTitle("Reddit");
        setSize(900,600);
        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        setVisible(true);
    }
 
    public static void setUpComponents(){
 
        JLabel redditIcon = new JLabel();
        redditIcon.setBounds(10, 10, 100, 100);
        resizeRedditLogo(redditIcon);
        mainPanel.add(redditIcon);
 
        textField.setBounds(160, 20, 250, 30);
        search.setBounds(415, 23, 85, 25);
        textField.setBackground(Color.decode("#8B0000"));
 
        search.setOpaque(true);
        search.setBorderPainted(false);
        search.setBackground(Color.decode("#8B0000"));
        search.setForeground(Color.WHITE);
        search.setBorder(BorderFactory.createRaisedBevelBorder());
 
        int y = 120;
 
        for (int i = 0; i < numbers.length;i++){
            numbers[i] = new JButton(String.valueOf(i+1));
            numbers[i].setBounds(20, y, 40,30);
            numbers[i].setBackground(Color.decode("#8B0000"));
            numbers[i].setOpaque(true);
            numbers[i].setForeground(Color.WHITE);
            numbers[i].setBorder(BorderFactory.createLoweredSoftBevelBorder());
 
            y+= 40;
 
            numbers[i].setBorderPainted(false);
            mainPanel.add(numbers[i]);
        }
 
        int y2 = 120;
 
        for (int i = 0; i < headlines.length;i++){
 
            headlines[i] = new JButton("");
            headlines[i].setBounds(70, y2, 650,30);
            headlines[i].setBackground(Color.decode("#8B0000"));
            headlines[i].setOpaque(true);
            headlines[i].setForeground(Color.WHITE);
            headlines[i].setBorder(BorderFactory.createLoweredSoftBevelBorder());
            y2+= 40;
            headlines[i].setBorderPainted(false);
            mainPanel.add(headlines[i]);
 
        }
 
        int y3 = 120;
 
        JLabel voteTitle = new JLabel("# Votes");
        voteTitle.setFont(new Font("Arial", Font.BOLD, 30));
        voteTitle.setForeground(Color.WHITE);
        voteTitle.setBounds(755, 70, 120, 30);
        mainPanel.add(voteTitle);
 
        for (int i = 0; i < votes.length;i++){
 
            votes[i] = new JLabel("");
            votes[i].setBounds(750, y3, 120,30);
            votes[i].setForeground(Color.WHITE);
            votes[i].setBorder(BorderFactory.createRaisedBevelBorder());
            y3+= 40;
            votes[i].setBorder(BorderFactory.createRaisedBevelBorder());
            mainPanel.add(votes[i]);
        }
 
 
        textField.setForeground(Color.WHITE);
 
        title.setBorder(BorderFactory.createRaisedBevelBorder());
        title.setBounds(105, 70, 605, 30);
        title.setFont(new Font("Arial", Font.BOLD, 18));
        title.setForeground(Color.WHITE);
        mainPanel.add(title);
        mainPanel.setBackground(Color.RED);
        mainPanel.add(textField);
        mainPanel.add(search);
 
    }
 
    public static void getSubReddit(String sub){
 
 
        try
        {
            Document doc = Jsoup.connect("https://old.reddit.com/r/"+sub).get();
            Elements el = doc.select("p.title");
            Elements score = doc.select("div.score.unvoted");
            int i,j;
 
            for (i=0,j=0;i<el.size() && j<score.size();i++,j++){
 
                headlines[i].setText(el.get(i).text());
                votes[j].setText(score.get(j).text());
            }
 
        } catch (Exception e) {
            System.out.println(e.toString());
        }
 
    }
 
    public static void resizeRedditLogo(JLabel label){
 
        try {
            ImageIcon imageIcon = new ImageIcon(new ImageIcon("../RedditGUI/src/reddit1.png").getImage().getScaledInstance(120, 120, Image.SCALE_DEFAULT));
            label.setIcon(imageIcon);
        } catch (Exception e) {
            e.printStackTrace();
        }
 
 
 
    }
    public static void main(String[] args) {
        new Reddit();
    }
 
 
\$\endgroup\$
6
  • \$\begingroup\$ Welcome to Code Review! While you provided a screenshot, it would benefit reviewers to have a bit more information about the code in the description. From the help center page How to ask: "You will get more insightful reviews if you not only provide your code, but also give an explanation of what it does. The more detail, the better." \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ May 4 at 17:39
  • \$\begingroup\$ @SᴀᴍOnᴇᴌᴀ thank you for the welcome and letting me know, do I just edit this post or I have to repost and edited version? \$\endgroup\$ – William Golovlev May 4 at 21:29
  • \$\begingroup\$ No problem. Please just edit the post. \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ May 4 at 21:30
  • \$\begingroup\$ There are way too many static fields and methods. The only static method in your whole Swing GUI should be the main method. Oracle has a nifty tutorial, Creating a GUI With JFC/Swing. Skip the Netbeans section. \$\endgroup\$ – Gilbert Le Blanc May 4 at 21:46
  • \$\begingroup\$ @GilbertLeBlanc thank you. Would you have any advise for the external appearance UI? \$\endgroup\$ – William Golovlev May 4 at 22:27
2
\$\begingroup\$

Intro. Sorry, I jumped in this question without taking time to read it in details. Since you are new there must be more to say in order to create an helpful answer. And I prefer to create a new one instead of editing the first one.


The easy way,

If you do not want to change so much:

  1. You can try to remove the static fields. Ideally, nothing should be static instead of the main entry point.

  2. You should also try to extract the parsing to a dedicated class. This will lead to a better separation of concerns and encapsulation and will let you test it or interchange it later.

  3. Create components. Individual components will clean-up and clarify the construction of your UI.

  4. Use layout managers to position the components.


The hard way,

So, yes. As said in my simpler answer and by Gilbert Le Blanc you should try to implement a pattern to separate the UI from the "logic". The MVC pattern is a common one in GUI applications, it is also the one used by advanced Swing components.

Then why not trying to create three classes:

  1. A model that represent the state of your application; the list of results.
  2. A view that contains the graphical components used to display the results from the model.
  3. A controller to receive actions from the view and update the model accordingly.

Model

Your model is barely a List<Subreddit> where Subreddit is a DTO with a title and a score. But your view should be notified when this list changes, that's why you have to create a dedicated class for it and add some boilerplate to register and notify one (or many) listeners.

View

Your view is your JFrame (or any Container displayed inside a JFrame). This view should listen register a listener on the model so that it can refresh it when the results changes.

class RedditAppView extends JFrame {
  public RedditAppView(RedditAppModel model) {
    model.addModelListener(new RedditAppModel.ModelAdapter(){
      public void onResultsChanged(List<Subbreddit> results) {
        refresh();
      }
    });
  }
}

It is easier to use a LayoutManager than using absolute layout in your views because the layout manager will take care of many part when you resize your application and when a component change. I like the GridBagLayout, it is a verbose layout (but Java and Swing are verbose too) but you can achieve almost everything with it and it is quite easy to reason about. However it can be very complex to create and maintain a large view. That's why we usually create components to encapsulate a functionality. In your case you can create:

  • A Logo that will take care of resizing itself.
  • A SearchForm that contains the JTextField and JButton add accept one event listener to be notified when the user click on the button, or press enter, or wait 3 seconds, .. that's one advantage of componentization; you can evolve one part of your app without touchhing the rest of the code.
  • A list of results. It can be a JList inside a JScrollPane. You can configure a custom renderer on that list to render the Subreddit in a more appealing way than the default toString.

Your view hierarchy may looks like:

RedditAppView
  |- Container
  |    |- Logo
  |    |- SearchForm
  |    |- ResultList
  |    |    |- JLabel ("# Votes")
  |    |    |- JScrollPane
  |    |    |    |- JList with Renderer

If you use the model as suggested, then you can create a class that adapt your RedditAppModel to a JListModel. By doing that you don't need to refresh by yourself, the list of result will be updated automatically:

class ResultListModelAdapter extends DefaultListModel<Subreddit> {
  private final RedditAppModel model;
  private ResultListModelAdapter(RedditAppModel model) {
    this.model = model;
    this.model.addModelListener(new RedditAppModel.ModelAdapter() {
      @Override
      public void onResultsChanged(List<Subreddit> results) {
        int end = Math.max(getSize(), results.size()); // Not sure it is needed
        fireContentsChanged(this, 0, end);
      }
    });
  }

  @Override
  public int getSize() {
    return model.getResultsCount()
  }

  @Override
  public Subreddit getElementAt(int index) {
    return model.getResultAt(index);
  }
}

Controller

A class that will handle user actions and reflect them on the model. Your controller will have one single search action called when the view receive.

class RedditAppController {
  private final RedditAppModel model;

  public RedditAppController(RedditAppModel model, RedditApi api) {
    this.model = model;
    this.api = api;
  }

  void onSearch(String term) {
    List<Subreddit> results = // Jsoup things
    model.setResults(results);
  }
}
\$\endgroup\$
1
\$\begingroup\$

About the code itself, you should try to separate the view from the "logic" by applying one of the various patterns (MVC and MVP being the most used), as said by Gilbert La Blanc in his comment,

About the visual part, it is easier to create components that provide high level functionality(ies) and that you use to compose your main view.

For the "layouting", why not using a JList with a custom renderer to display each line. If you need clickable buttons (that are not implemented as far as I can see in your code) then you can use a Box or a BoxLayout.

\$\endgroup\$

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.