HiveBrain v1.2.0
Get Started
← Back to all entries
snippetjavaMinor

Monitor, read and display data from a dynamic log file

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
fromfilereadlogdynamicmonitoranddisplaydata

Problem

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[]= {"header

Solution

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.

Code Snippets

Thread.sleep(1000);
while(true){
private final String headers[]= {"header1","header2","header3","header4","header5","header6"};
private ToolWindow toolWindow;
//reads the log file to retrieve data.

//connect FileReader to log file

Context

StackExchange Code Review Q#112717, answer score: 2

Revisions (0)

No revisions yet.