patternjavaMinor
Simple servlet code to sum digits
Viewed 0 times
simpledigitsservletsumcode
Problem
This servlet takes an input from the user and
I can't help thinking that this code is too long and can be shorter and better. How can I improve this code?
- Stores the number inside the bean but with spaces in between each digit then stores the bean in the request scope.
- If the last digit is even, sums the numbers and stores it in the request attribute. It will be displayed by even.jsp. Otherwise, the servlet forwards to odd.jsp and just displays the numbers.
protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
numberBean bean = new numberBean();
int sum = 0;
char[] inputNumbers = request.getParameter("input").toCharArray();
StringBuilder inputs = new StringBuilder();
for (char digit: inputNumbers) {
inputs.append(digit).append(" ");
}
bean.setInput(inputs.toString());
request.setAttribute("input", bean);
if (inputNumbers[inputNumbers.length - 1] % 2 == 0) {
for (char digit: inputNumbers) {
sum += Character.getNumericValue(digit);
}
request.setAttribute("sum", sum);
request.getRequestDispatcher("/WEB-INF/even.jsp").forward(request, response);
} else {
request.getRequestDispatcher("/WEB-INF/odd.jsp").forward(request, response);
}
}I can't help thinking that this code is too long and can be shorter and better. How can I improve this code?
Solution
I am not aware of how to improve the code so that it will be shorter. What I would suggest is that this code can be improved in terms of clarity of intent (in this case, the LOCs will increase). This can be achieved by using small functions and good names for the functions (for more information look and Robert C. Martins wonderful book: Clean Code or this for a little introduction).
I refactored your code by extracting some methods and giving more or less meaningful names:
One could argue about the existence about some of the methods like
In addition, the class
I refactored your code by extracting some methods and giving more or less meaningful names:
protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
char[] inputNumbers = extractInputNumbersFromRequest(request);
NumberBean bean = createNumberBeanForInputNumbers(inputNumbers);
request.setAttribute("input", bean);
forwardRequestToCorrectView(request, response, inputNumbers);
}
private char[] extractInputNumbersFromRequest(HttpServletRequest request) {
return request.getParameter("input").toCharArray();
}
private NumberBean createNumberBeanForInputNumbers(char[] inputNumbers) {
NumberBean bean = new NumberBean();
String inputNumbersWithSpaces = combineInputNumbersWithSpaces(inputNumbers);
bean.setInput(inputNumbersWithSpaces);
return bean;
}
private String combineInputNumbersWithSpaces(char[] inputNumbers) {
StringBuilder inputs = new StringBuilder();
for (char digit: inputNumbers) {
inputs.append(digit).append(" ");
}
return inputs.toString();
}
private void forwardRequestToCorrectView(HttpServletRequest request, HttpServletResponse response, char[] inputNumbers) throws ServletException, IOException {
if (lastNumberIsOdd(inputNumbers)) {
handleLastNumberOddCase(request, response, inputNumbers);
} else {
handleLastNumberEvenCase(request, response);
}
}
private void handleLastNumberEvenCase(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
request.getRequestDispatcher("/WEB-INF/odd.jsp").forward(request, response);
}
private void handleLastNumberOddCase(HttpServletRequest request, HttpServletResponse response, char[] inputNumbers) throws ServletException, IOException {
request.setAttribute("sum", sumOfInputNumbers(inputNumbers));
request.getRequestDispatcher("/WEB-INF/even.jsp").forward(request, response);
}
private int sumOfInputNumbers(char[] inputNumbers) {
int sum = 0;
for (char digit: inputNumbers) {
sum += Character.getNumericValue(digit);
}
return sum;
}
private boolean lastNumberIsOdd(char[] inputNumbers) {
return inputNumbers[inputNumbers.length - 1] % 2 == 0;
}One could argue about the existence about some of the methods like
handleLastNumberEvenCase, but I like the idea from Uncle Bob, that a method should not have different levels of abstractions. As such, I've introduced these alias-like methods, which just give a one liner a name, that is more clear.In addition, the class
numberBean was renamed to NumberBean to use the standard conventions in the Java world. Furthermore, it should be renamed to a more meaningful name, because the word "Number" does not give you a hint on what this class does. The word "Bean" does not give you further information as well, so I would suggest something like WhitespaceNumberRequestor - which is obviously false, because I don't have any hints on what this class does.Code Snippets
protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
char[] inputNumbers = extractInputNumbersFromRequest(request);
NumberBean bean = createNumberBeanForInputNumbers(inputNumbers);
request.setAttribute("input", bean);
forwardRequestToCorrectView(request, response, inputNumbers);
}
private char[] extractInputNumbersFromRequest(HttpServletRequest request) {
return request.getParameter("input").toCharArray();
}
private NumberBean createNumberBeanForInputNumbers(char[] inputNumbers) {
NumberBean bean = new NumberBean();
String inputNumbersWithSpaces = combineInputNumbersWithSpaces(inputNumbers);
bean.setInput(inputNumbersWithSpaces);
return bean;
}
private String combineInputNumbersWithSpaces(char[] inputNumbers) {
StringBuilder inputs = new StringBuilder();
for (char digit: inputNumbers) {
inputs.append(digit).append(" ");
}
return inputs.toString();
}
private void forwardRequestToCorrectView(HttpServletRequest request, HttpServletResponse response, char[] inputNumbers) throws ServletException, IOException {
if (lastNumberIsOdd(inputNumbers)) {
handleLastNumberOddCase(request, response, inputNumbers);
} else {
handleLastNumberEvenCase(request, response);
}
}
private void handleLastNumberEvenCase(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
request.getRequestDispatcher("/WEB-INF/odd.jsp").forward(request, response);
}
private void handleLastNumberOddCase(HttpServletRequest request, HttpServletResponse response, char[] inputNumbers) throws ServletException, IOException {
request.setAttribute("sum", sumOfInputNumbers(inputNumbers));
request.getRequestDispatcher("/WEB-INF/even.jsp").forward(request, response);
}
private int sumOfInputNumbers(char[] inputNumbers) {
int sum = 0;
for (char digit: inputNumbers) {
sum += Character.getNumericValue(digit);
}
return sum;
}
private boolean lastNumberIsOdd(char[] inputNumbers) {
return inputNumbers[inputNumbers.length - 1] % 2 == 0;
}Context
StackExchange Code Review Q#53993, answer score: 6
Revisions (0)
No revisions yet.