patternjavaMinor
Check smsc type of batch and submits it to the proper queue
Viewed 0 times
theproperbatchsmsctypesubmitsandcheckqueue
Problem
Here, the for loop loops over a arraylist and checks smsc type of batch and submits it to the proper queue.
Is there a way to rewrite this piece of code in a better way? Is there any way to eliminate the if/else?
Is there a way to rewrite this piece of code in a better way? Is there any way to eliminate the if/else?
for (Batch batch : input.getBatches()){
ArrayList arrSentsms = batch.getlist();
String smsc =batch.getSmsc();
if (smsc.equals("v")) {
SubmitToQueueV.sendJMSMessage(arrSentsms, priority);
} else if (smsc.equals("s")) {
SubmitToQueueS.sendJMSMessage(arrSentsms, priority);
} else if (smsc.equals("fake1")) {
SubmitToQueueFake.sendJMSMessage(arrSentsms, priority);
} else if (smsc.equals("con")) {
SubmitToQueueCon.sendJMSMessage(arrSentsms, priority);
} else if (smsc.equals("ce")) {
SubmitToQueueCEcns.sendJMSMessage(arrSentsms, priority);
} else if (smsc.equals("cli")) {
SubmitToxQueueCli.sendJMSMessage(arrSentsms, priority);
} else if (smsc.equals("pel")) {
SubmitTQueuePel.sendJMSMessage(arrSentsms, priority);
} else {
LOG.debug("smsc" + " smsc not found");
}
}
}Solution
I assume you have (or can easily extract) an interface JMSQueue for this answer.
That's not really an unreasonable assumption I'd say, if you need help with that just drop me a note.
You're using the wrong tool here. What you have is a "mapping" between a string and a certain JMS-Queue.
Let's put that into code:
Note that I've done some small changes on your code:
That's not really an unreasonable assumption I'd say, if you need help with that just drop me a note.
You're using the wrong tool here. What you have is a "mapping" between a string and a certain JMS-Queue.
Let's put that into code:
private Map queues = new HashMap<>();
// [...]
for (Batch batch : input.getBatches()) {
JMSQueue responsibleQueue = queues.get(batch.getSmsc());
if (responsibleQueue == null) {
// queue not found. Log or whatever and then do the next batch
continue;
}
responsibleQueue.sendJMSMessage(batch.getList(), priority);
}Note that I've done some small changes on your code:
- Respect indentation and whitespace around operators. Your IDE can help you with that.
- Avoid unnecessary variables. They only make it harder to understand the code. Don't have everything inlined, but exercise care when creating variables.
- Respect the naming conventions (talking about
getlist)
Code Snippets
private Map<String, JMSQueue> queues = new HashMap<>();
// [...]
for (Batch batch : input.getBatches()) {
JMSQueue responsibleQueue = queues.get(batch.getSmsc());
if (responsibleQueue == null) {
// queue not found. Log or whatever and then do the next batch
continue;
}
responsibleQueue.sendJMSMessage(batch.getList(), priority);
}Context
StackExchange Code Review Q#139714, answer score: 6
Revisions (0)
No revisions yet.