Is the code properly refactored or still some more improvements needs to be done?

I have got code here which I have tried to refactor on my own - but do any of you experts feel that it needs to be refactored more? What are the points should I cover? I would appreciate it if some one gives me some points on this code review.

```
package com.vilant.app.outbound;

import java.util.ArrayList;
import java.util.Collections;
import java.util.Date;
import java.util.List;

import javax.persistence.EntityManagerFactory;
import javax.persistence.Query;

import com.vilant.app.db.Container;
import com.vilant.app.db.OutboundQueue;
import com.vilant.app.db.Shipment;
import com.vilant.util.jpa.EntityManagerController;
import com.vilant.util.jpa.ThreadLocalEntityManager;

/ TODO: responsibility resolvation through database is far from bulletproof... /

// TODO: implement HA negotiation and failover + message passing to active sender
public class OutboundSender implements Runnable {

private org.slf4j.Logger log = org.slf4j.LoggerFactory.getLogger(this.getClass());

// minutes after next attempt time when to acquire queue item from db
private int acquireThreshold = 120; // TODO: make configurable

// database orphaned queue item check interval
private int dbCheckInterval = 120; // TODO: make configurable

private Date nextDbCheck = new Date();

private ThreadLocalEntityManager tem;
private EntityManagerController emc;

private List attemptQueue = Collections.synchronizedList(new ArrayList());
private Thread runnerThread;
private OutboundSender runner;
private boolean running = true;

private static OutboundSender instance;
private OutboundSender() {

}
public void initPersistence(EntityManagerFactory emf) {
tem = new ThreadLocalEntityManager(emf);
emc = new EntityManagerController(tem);
}

public static OutboundSender getInstance() {
synchronized(OutboundSender.class) {
if (instance==null) {
instance = new OutboundSender();
}

First , It seems that this code violates Single Responsibility Principle and it way too long to do just a single task .If that's the case maybe you can introduce new small classes that assist this class in doing its task.
Second , I can see signs of DRY here and there specially in performDbCheck :

here :

if (q.getIdType()==OutboundQueue.IdentifierType.CONTAINER_ID) {
                        Container container = tem.find(Container.class, q.getIdValue());
                        if (container!=null) {
                            emc.beginTx();
                            long acquireValue = (long)(Math.random()*Long.MAX_VALUE);
                            q.setHandler(acquireValue);
                            tem.merge(q);
                            emc.commitTx();
                            this.enqueueContainerRelatedMessage(container, handlerClass, q.getParameter(), q.getId(), acquireValue, q.getAttempts());
                        }
                        else {
                            log.error("Container has disappeared from database, removing queue item");
                            emc.beginTx();
                            tem.merge(q);
                            tem.remove(q);
                            emc.commitTx();
                        }
                    }
                    else if

You are doing the same thing in several conditions like beginTx(),tem.merge(q) and ....
and there are only a few exceptional line of codes in between. You can have a method that accept some callback function to do the general task + excpetional code:

for example :

public void RunTransaction(IExceptionalCode code)
{
beginTran();
merge();
code.DoTheTask();
endTran();
}

then you can have different classes implementing IExceptionalCode doing specific task:
3.There are a lot of nested if s in your code that make your code hard to read and maintain.You can have a Rule object that has a condition and an IExceptionalCode objct in it

public Rule 
{
private ICondition _condition;

private IExceptionalCode_logic;

public Rule(ICondition condition,IExceptioanlCode code)
{
_condition=conditionl
_logic=code;
}

public void Apply(parameters to be check)
{
if(_condition.IsTrue(parameters to be check) RunTransaction(_logic);
}
}

then you can have a list of rules in your method and :

foreach(Rule rule in rules)
rule.Apply(parameters);

This way you can add more logic to your code later very easily by introducing new rules.

StackExchange Code Review Q#9297, answer score: 7