Java Application Design

Question

I am pretty new to Object Programming and Java, so I am here to gather your advice and feedback. Basically I am trying to write a background service which performs different tasks at different intervals. I'm just not 100% sure of what I am doing is following the coding standards or is efficient.

Main / Start Class:

public class Start {

    public static void main(String[] args) {
        Service s = new Service();
        s.Start();
    }

}

Database Class:

import java.sql.Connection;
import java.sql.DriverManager;
import java.sql.SQLException;

public class Database {

    /* Database settings */
    private final String HOSTNAME = "localhost";
    private final String DATABASE = "java_database";
    private final String USERNAME = "java_username";
    private final String PASSWORD = "java_password";

    /* Database connection */
    public Connection getConnection() {
        try {
            return DriverManager.getConnection("jdbc:mysql://" + HOSTNAME + "/" + DATABASE + "?user=" + USERNAME + "&password=" + PASSWORD + "&useSSL=false&useUnicode=true&characterSetResults=utf8");
        } catch (SQLException ex) {
            ex.printStackTrace();
        }
        return null;
    }


}

Service Class:

import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;

public class Service {

    private int taskId;
    private int taskType;


    /* Start Service */
    public void Start() {
        try {
            System.out.println("Starting Service...");
            while(true) {
                System.out.print("Checking for tasks... ");
                getNextTask();
                if (this.taskId > 0) {
                    System.out.println("Task ID " + this.taskId + " found.");
                    switch (this.taskType) {
                        case 1:
                            System.out.println("Task 1");
                            SampleTask s = new SampleTask();
                            s.Start();
                            s = null;
                            break;
                        default:
                            System.out.println("Error: Unknown Task");
                    }                   
                    setUsedTask();
                } else {
                    System.out.println("No tasks to perform at this time.");
                }
                this.taskId = 0;
                this.taskType = 0;
                Thread.sleep(5000);
            }
        } catch (InterruptedException ex) {
            ex.printStackTrace();
        }
    }


    /* Gets the next available task from the database */
    public void getNextTask() {
        try {
            Database db = new Database();
            String query = "select taskId, taskType "
                    + "from tasks "
                    + "where (time_to_sec(timediff(now(), taskLastRun)) > taskFrequency or taskLastRun is null) and taskEnabled = 1 "
                    + "limit 1";
            Statement stmt = db.getConnection().createStatement();
            ResultSet rset = stmt.executeQuery(query);
            if (rset.next()) {
                this.taskId = rset.getInt(1);
                this.taskType = rset.getInt(2);
            }
        } catch (SQLException ex) {
            ex.printStackTrace();
        }
    }


    /* Set task as complete */
    public void setUsedTask() {
        try {
            Database db = new Database();
            String query = "update tasks "
                    + "set taskLastRun = now() "
                    + "where taskId = ? "
                    + "limit 1";
            PreparedStatement pstmt = db.getConnection().prepareStatement(query);
            pstmt.setInt(1, this.taskId);
            pstmt.executeUpdate();
        } catch (SQLException ex) {
            ex.printStackTrace();
        }
    }


}

Do not use `Thread.seep()`. Use `ScheduledExecutorService` instead — Svetlin Zarev, Mar 08 '17 at 16:38
I'm voting to close this question as off-topic because questions asking to review working code should go to http://codereview.stackexchange.com/ — GhostCat, Mar 09 '17 at 09:20
@Zoe Feel free to recommend the OP post on CR but in the future, please don't use Code Review as a reason to close a question. Evaluate the request and use a reason like *too broad*, *primarily opinion-based*, etc. Then you can mention to the OP that it can be posted on Code Review if it is [on-topic](https://codereview.stackexchange.com/help/on-topic). Please see the section **What you should not do** in [this answer to _A guide to Code Review for Stack Overflow users_](https://codereview.meta.stackexchange.com/a/5778/120114) — Sᴀᴍ Onᴇᴌᴀ, Nov 13 '18 at 16:26
@SᴀᴍOnᴇᴌᴀ It's still off-topic on Stack Overflow, and considering CR was recommended earlier, I used that over TB. — Zoe, Nov 13 '18 at 16:42
@Zoe I agree that it is still off topic... if you look at that post I linked you would see it contains the following: "_Please do not vote to close with a custom reason that "it belongs on Code Review". Nothing in the Stack Overflow rules justifies such a custom reason, and sloppy reasoning perpetuates inappropriate referrals. Not all questions about analyzing code are off-topic on Stack Overflow, and not all code review requests are on-topic on Code Review. Instead, vote to close as too broad or primarily opinion-based._" and then goes on to mention the GUI being misleading... — Sᴀᴍ Onᴇᴌᴀ, Nov 13 '18 at 16:44

score 0 · Answer 1 · edited May 23 '17 at 12:02

Consider replacing your Thread.sleep() approach with a wait() and notify() approach as discussed here.

public class Service {

    private int taskId;
    private int taskType;
    private final Object serviceMonitor;


    /* Start Service */
    public void Start() {
        synchronized(serviceMonitor){
          try {
            System.out.println("Starting Service...");
            while(true) {
                System.out.print("Checking for tasks... ");
                getNextTask();
                if (this.taskId > 0) {
                    System.out.println("Task ID " + this.taskId + " found.");
                    switch (this.taskType) {
                        case 1:
                            System.out.println("Task 1");
                            SampleTask s = new SampleTask();
                            s.Start();
                            s = null;
                            break;
                        default:
                            System.out.println("Error: Unknown Task");
                    }                   
                    setUsedTask();
                } else {
                    System.out.println("No tasks to perform at this time.");
                }
                this.taskId = 0;
                this.taskType = 0;
                serviceMonitor.wait();
            }
          }
        }
        catch (InterruptedException ex) {
            ex.printStackTrace();
        }
    }

public void getNextTask() {
    synchronized(serviceMonitor){
    try {
        Database db = new Database();
        String query = "select taskId, taskType "
                + "from tasks "
                + "where (time_to_sec(timediff(now(), taskLastRun)) > taskFrequency or taskLastRun is null) and taskEnabled = 1 "
                + "limit 1";
        Statement stmt = db.getConnection().createStatement();
        ResultSet rset = stmt.executeQuery(query);
        if (rset.next()) {
            this.taskId = rset.getInt(1);
            this.taskType = rset.getInt(2);
            serviceMonitor.notifyAll();
        }
      }
    } catch (SQLException ex) {
        ex.printStackTrace();
    }
}

score 0 · Answer 2 · answered Mar 11 '17 at 15:44

public class Main {
    public static void main(String[] args) {
        Service service = new Service(Arrays.asList(new SampleTask(),new AnotherTask()));
        service.execute();
   }
}



class Service {
     private List<Task> taskList;

     public Service(List<Task> taskList) {
        this.taskList = taskList;
     }
     public void addTask(Task task) {
         taskList.add(task);
     }
     public void execute() {
         for (Task task : taskList) {
             new Timer().schedule(task, task.getDelay(), task.getPeriod());
         }
     }
     public void clearTasks() {
         taskList.clear();
     }
}


abstract class Task extends TimerTask {
     abstract long getDelay();
     abstract long getPeriod();
}


class SampleTask extends Task {
    public void run() {
        System.out.println("Sample task executed");
    }

    long getDelay() {
        return 1000;
    }

    long getPeriod() {
        return 60000;
    }
 }

class AnotherTask extends Task {
    public void run() {
        System.out.println("Another task is executed");
    }

    long getDelay() {
        return 1000;
    }

    long getPeriod() {
       return 500000;
    }
}

Java Application Design

2 Answers2