3
\$\begingroup\$

I am developing a plugin for intellij that basically monitors a log file and displays the data in a tool window within the IDE in real-time. I have managed to get it working but I would like to know the areas where I can improve on. I'm specifically interested in the structure of my code and the log file monitoring method followed.

The createToolWindowContent method is called when the log panel is opened for the first time.

public class LogToolWindowLoader implements ToolWindowFactory {

private UIOperations ui;
private LogFileFunctions log_func;
private ToolWindow toolWindow;
@Override
public void createToolWindowContent(Project project, ToolWindow toolWindow) {

    this.toolWindow = toolWindow;
    //this generates the log viewer GRID within the IDE
    ui = new UIOperations(toolWindow);
    ui.createTable();



    //Create new thread to retrieve data from log file. this is a continuous process, hence the new Thread
    new Thread(){
        public void run(){

            log_func = new LogFileFunctions();

            log_func.connectToLog();

            while(true){
                String[] result = log_func.getData();
                if (result!=null) {
                    //GUI updated on the Event Dispatcher Thread
                    SwingUtilities.invokeLater(new Runnable() {
                        @Override
                        public void run() {
                            ui.updateTable(result);
                        }
                    });
                }
                else {
                    try {
                        Thread.sleep(1000);
                    } catch (InterruptedException e) {
                        e.printStackTrace();
                    }
                }
            }
        }
    }.start();}

UI operations are handled in this class:

public class UIOperations {

private ToolWindow toolWindow;
private DefaultTableModel table_model;
private JBTable table;
private final String headers[]= {"header1","header2","header3","header4","header5","header6"};

public UIOperations(ToolWindow toolWindow){
    this.toolWindow = toolWindow;
}


public void createTable(){

    JComponent panel = new JPanel(new BorderLayout()); //new BorderLayout extends panel completely inside the tool Window
    panel.setBackground(JBColor.WHITE);
    toolWindow.getComponent().add(panel);

    //set column names
    table_model = new DefaultTableModel(headers,0);
    table = new JBTable(table_model);

    JScrollPane scrollPane = new JBScrollPane(table);
    panel.add(scrollPane);

    //table formatting
    table.setShowHorizontalLines(false);
    table.setShowVerticalLines(false);
    table.setFillsViewportHeight(true);//sets table background colour

    System.out.println("Log Viewer Table Generated..");

}

public void updateTable(String[] str){

    table_model.addRow(str);

}
}

Log file processing is handled in this class:

public class LogFileFunctions {

private BufferedReader in;

//reads the log file to retrieve data.
public String[] getData(){

    //System.out.println(System.getProperty("user.dir")); gives the executing directory
    String str; //holds string read from file
    String[] result=null; //holds tokenized string

    try {
        str = in.readLine();

        if (!(str == null)) {
            result = str.split("[|]");
        }

    }catch (FileNotFoundException e) {
        e.printStackTrace();
    } catch (IOException e) {
        e.printStackTrace();
    }

    return result;
}


//connect FileReader to log file
public void connectToLog() {

    try {
        FileReader fileReader = new FileReader(getClass().getResource("log.txt").getPath());
        in = new BufferedReader(fileReader);

        System.out.println("Log File location: " + getClass().getResource("log.txt").getPath());
        } catch (FileNotFoundException e) {
            System.out.println("ERROR: log.txt file not found at " + getClass().getResource("log.txt").getPath() );
        }
    }
}

EDIT: I would appreciate pointers on how to optimize my code

\$\endgroup\$
0

1 Answer 1

2
\$\begingroup\$

LogToolWindowLoader.java

Constant or user property

Thread.sleep(1000);

This is a nice example of magic number.
It's better that you create at least a private static final int for it, but even better could be the possibility to change that value by a property file, with a default of 1000 if the property isn't there.
Like this you could give the users easily control that value, because in mine opinion there could be people who want it slower or faster.

To the infinitive

while(true){

I never like to see this hack.
It's a infinitive loop with no possibility to stop it.
Why not make it a variable and give the user a possibility to stop loop?

UIOperations.java

Constants

private final String headers[]= {"header1","header2","header3","header4","header5","header6"};

A good usage of the final word and array but you create this array for each instantiation of the class.
Better could make that field also static so you have instantiated this variable once for all the different instantiations of UIOperations.

Final keyword in other situations

private ToolWindow toolWindow;

This field could be final because you set it in the constructor and there is no setter for setting this at a later point.

LogFileFunctions.java

Javadoc preffered

//reads the log file to retrieve data.

//connect FileReader to log file

This could be better set as java doc in stead of normal comments.
Like this if you generate the javadoc, people who read that will also see your comment.

same or not the same

if (!(str == null)) {

What the hell?
I hope this is old code created by resolving problems because otherwise this is actually your biggest issue (In mine personally insight).
For the moment you check if it's equal and then reverse the result.
There is nothing wrong with just checking for not equal :

if (str != null) {

Naming

String str; //holds string read from file
String[] result=null; //holds tokenized string

Code should be self explaining, so if you need to put comment behind a variable name to explain it, there is something wrong.
Mine suggestion is :

String lineFromFile;
String tokenizedLine=null;

Opening and closing resources

As last in this class is for me the absence of closing your resources.
This is just a nice example to create memoryleaks.

Summary

You see you have good experience in programming, it's a nice and readable code with really tiny things I could point out.
I hope this review could help you in one or more points.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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