Review of document registrar Would you be kind to make me a code review? It would be nice to have some comments, whether you would have done something differently and so on.  I am not sure about the way I handle the exception.  Is there a better way?

@Transactional
public classServiceImpl implements Service {

    private static final Logger LOG = Logger.getLogger(HermesServiceImpl.class);


    @Autowired(required = true)
    private Client client;

    @Autowired
    private SubmissionDAO submissionDAO;

    @Autowired
    private SubmissionSender hrsQueueSender;

    @Transactional
    @Override
    public void registerDocument(String submissionId) {

        final long start = System.currentTimeMillis();
        final List&lt;HermesItem&gt; items = submissionDAO.getSubmissionItems(submissionId);
        RegistrationSummary regSummary = null;

        if (!items.isEmpty()) {
            final HrsDocumentBuilder builder = new HrsDocumentBuilder();
            final Date docDate = new Date();
            builder.setTitle(submissionId);
            builder.setDocumentDate(docDate);
            builder.setSentDate(docDate);
            builder.setMailType(HermesMailType.INTERNAL);

            /* Document is constructed, try to invoke Hermes - upload annexes and register document. */
            try {
                regSummary = client.registerDocument(builder.buildDocument());
            } catch (final ClientException e) {
                LOG.error("Unable to register document with Submission Id= [" + submissionId + "], " + e.getMessage());
            }
            if (regSummary != null) {
                final String hermesDocId = regSummary.getHermesDocumentId();
                /* Update the DB with the documentID received from Hermes. */
                final int update = submissionDAO.updateSubmission(submissionId, hermesDocId, SubmissionStatus.HRS_REGISTERED.name());

                if (update &lt; 1) {
                LOG.error("update error");
                }
            } else {
                submissionDAO.updateSubmission(submissionId, null, SubmissionStatus.HRS_FAILED.name());
                hrsQueueSender.sendMesage(submissionId);
            }

        } else {
            LOG.error("Not found");
            submissionDAO.updateSubmission(submissionId, null, SubmissionStatus.ANNEXES_NOT_FOUND.name());
        }
    }
}
Few points:


Try to avoid long or nested if/else 
Sub-divide tasks in smaller functions(clean and more readable code + easier unit testing)
Minimize variable scope(try to declare variables in the scope where you really need them)
Think about Exception in context of your business domain, may be using builder pattern (like HrsDocumentBuilder) can be useful to have consistent logging message for same type of exception(override toString() method).


We can refactor the method registerDocument() like this:

You may replace  

final long start = System.currentTimeMillis();


with StopWatch

@Transactional
@Override
public void registerDocument(String submissionId) {
final long start = System.currentTimeMillis();    
final List&lt;HermesItem&gt; items = submissionDAO.getSubmissionItems(submissionId);

if (!hasItems(items)) //extract else to method
    return;

final HrsDocumentBuilder builder = ...builder.setMailType(HermesMailType.INTERNAL);//stays the same
try {
      // minimized the scope of local variable, you need regSummary in try block so declare it here.
        RegistrationSummary regSummary = client.registerDocument(builder.buildDocument()); // will rename buildDocument() -&gt; build() 
        update(regSummary);

} catch (final ClientException e) { 

       //To standarized log message we can refactor it like :
        ClientExceptionBuilder b = new ClientExceptionBuilder(e).submissionId(submissionId).build();// override toString()
        LOG.error(b.getMessage());
}
}//function registerDocument() end here


Since all other method can be easily extracted from else blocks, i am posting only the update(regSummary) because this function can be further subdivide into two smaller functions like this:

void update(regSummary) {

    if(!hasSummary()) //extract else to method 
        return 

    final String hermesDocId = regSummary.getHermesDocumentId();
    /* Update the DB with the documentID received from Hermes. */
    final int update = submissionDAO.updateSubmission(submissionId, hermesDocId, SubmissionStatus.HRS_REGISTERED.name());

    if (update &lt; 1) 
        LOG.error("update error");

}