HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

A simple web-app code - user registration. Is the layering ok?

Submitted by: @import:stackexchange-codereview··
0
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

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:

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.