patternjavaMinor
A simple web-app code - user registration. Is the layering ok?
Viewed 0 times
simpletheregistrationappuserwebcodelayering
Problem
So knowing the little pieces is something different and putting them together is something different.
Here is the code where I try to follow good oop practices and 3 - layered structure.
Any review is greatly appreciated:
The form is located at registirationform.jsp.
The view layer:
```
public class RegistirationServlet extends HttpServlet {
@Override
protected void doPost(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse)
throws ServletException, IOException {
httpServletRequest.setCharacterEncoding("utf-8");
httpServletResponse.setContentType("text/html;charset=utf-8");
try {
UserRegistirationService userRegistirationService
= new UserRegistirationService(new AppUserAccessObject(new DatabaseConnectionImpl()));
String username = httpServletRequest.getParameter("username");
String password = httpServletRequest.getParameter("password");
String email = httpServletRequest.getParameter("email");
if(username.equals("")
|| password.equals("")
|| email.equals("")){
List errors = new ArrayList();
errors.add("Can you make sure you fill all the fields?");
httpServletRequest.setAttribute("errors",errors);
if(!username.equals("")){
httpServletRequest.setAttribute("username",username);
}
if(!email.equals("")){
httpServletRequest.setAttribute("email",email);
}
httpServletRequest.getRequestDispatcher("/registirationform.jsp").forward(httpServletRequest, httpServletResponse);
}
else {
userRegistirationService.registerAppUser(username, password, email);
}
} catch (SQLException e) {
e.printStackTrace();
List errors = new Ar
Here is the code where I try to follow good oop practices and 3 - layered structure.
Any review is greatly appreciated:
The form is located at registirationform.jsp.
The view layer:
```
public class RegistirationServlet extends HttpServlet {
@Override
protected void doPost(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse)
throws ServletException, IOException {
httpServletRequest.setCharacterEncoding("utf-8");
httpServletResponse.setContentType("text/html;charset=utf-8");
try {
UserRegistirationService userRegistirationService
= new UserRegistirationService(new AppUserAccessObject(new DatabaseConnectionImpl()));
String username = httpServletRequest.getParameter("username");
String password = httpServletRequest.getParameter("password");
String email = httpServletRequest.getParameter("email");
if(username.equals("")
|| password.equals("")
|| email.equals("")){
List errors = new ArrayList();
errors.add("Can you make sure you fill all the fields?");
httpServletRequest.setAttribute("errors",errors);
if(!username.equals("")){
httpServletRequest.setAttribute("username",username);
}
if(!email.equals("")){
httpServletRequest.setAttribute("email",email);
}
httpServletRequest.getRequestDispatcher("/registirationform.jsp").forward(httpServletRequest, httpServletResponse);
}
else {
userRegistirationService.registerAppUser(username, password, email);
}
} catch (SQLException e) {
e.printStackTrace();
List errors = new Ar
Solution
I know you're asking strictly for feedback about the layers (which look fine to me), but in case you're looking for other feedback too:
You need to null check these values.
Can be done in one line if you want:
if(username.equals("")
|| password.equals("")
|| email.equals("")){You need to null check these values.
getParameter can return null so you will get a NullPointerException if they don't exist.List errors = new ArrayList();
errors.add("Registiration was not successful. Maybe username/email was already taken?");
httpServletRequest.setAttribute("errors",errors);Can be done in one line if you want:
errors.add(Arrays.asList("Registiration was not successful. Maybe username/email was already taken?"));Code Snippets
if(username.equals("")
|| password.equals("")
|| email.equals("")){List<String> errors = new ArrayList<String>();
errors.add("Registiration was not successful. Maybe username/email was already taken?");
httpServletRequest.setAttribute("errors",errors);errors.add(Arrays.asList("Registiration was not successful. Maybe username/email was already taken?"));Context
StackExchange Code Review Q#47507, answer score: 3
Revisions (0)
No revisions yet.