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

Confidential Sign-In Sheet in Java

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
confidentialsignsheetjava

Problem

I was requested to make a Sign-in program for the LGBTA Center at my university. They wanted something where anyone that came by and visited could leave their name and email, but also so that no one that came behind them could see who else has come in, like an Excel sheet or regular piece of paper would do.

The program takes an email (which is checked to see if it contains an '@' and ends with the five most common extensions) and a name, generates the exact time and date it was submitted, and writes all that information to a .txt file. If that .txt file does not exist, a new one is created with that day's date as the file name; otherwise, it appends to the correct file.

I've tried to make it as user-friendly as possible, such as pressing enter in the text field for name requests focus on the text field for email. Pressing enter in the text field for email will check if everything is valid, and if so, writes to the file.

Right now, the files are saved in the same location as the .jar file I package it in, but this is only because the program is going on a university computer where anyone (the computer it is going on is used for work study by different students) that signs in might need access to it. I'm not sure if there's a better way to do this as I'm not that familiar with the file system of a Mac.

CenterSignIn.java (main)

import javax.swing.JFrame;

public class CenterSignIn 
{   
    public static void main(String[] args)
    {
        JFrame frame = new JFrame("Center Sign In");
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.getContentPane().add(new CenterGUI());
        frame.pack();
        frame.setResizable(false);
        frame.setLocationRelativeTo(null);
        frame.setVisible(true);
    }
}


CenterGUI

```
import java.awt.*;
import java.awt.event.*;
import javax.swing.*;
import java.io.*;
import javax.swing.border.EmptyBorder;
import java.util.Date;
import java.text.SimpleDateFormat;

Solution

Composition over inheritance

In GUI, specially Swing, prefer composition over inheritance. You don't need to override any of the normal behaviour, so just create a normal JPanel and just remove the extends.

Extending just make things weird since now your class is a lot of things it should be. A normal JPanel doesn't have field for specific things in it.

Use braces

I always suggest the use of {} even when you have only one line. It lower the number of changes if you add another line of code, it leave the code in a consistent state (braces everywhere) and you eliminate a possible bug in future code.

Exception

If you don't want to deal with IOExcpetion and just swallow the exception, why do you declare the method with throws IOException ?

try {
            initFile();
        } catch ( IOException e ) {}


The method declare a new file if the file doesn't exist, so you could just remove the checked exception and just deal with the error in the method directly.

Constant

Always look for duplicated piece when you're done with your code. Is there some String content that I could refactored in a constant? Are there repetitive piece of code that I could create a method ?

Here you have new Font("Serif", Font.PLAIN, 18) that is re-declared almost everywhere for each label. Why don't you declare a constant, so when you want to change the font you only have one change to do :)?

Naming

Please follow the official naming convention of Java, private class listener implements ActionListener, KeyListener should be private class Listener implements ActionListener, KeyListener.

Naming is hard too, Listener is so generic that it could be anything at that point. In your code that class has a very specific role, it deal with the action done in this view. It could be named CenterGUIListener or something more specific.

Named condition

When I deal with multiple || and && in a condition, I like to make a method out of the complicated condition.

if(!e.contains("@") || (!e.endsWith(".com") && !e.endsWith(".org") && !e.endsWith(".edu") && !e.endsWith(".gov") && !e.endsWith(".net")))


Here you could have the method called isEmailAndHaveValidExtensions. Most of the time, I prefer to have a named method that will tell me what it validate and if needed I'll check the method to see how it enforce that validation.

Code Snippets

try {
            initFile();
        } catch ( IOException e ) {}
if(!e.contains("@") || (!e.endsWith(".com") && !e.endsWith(".org") && !e.endsWith(".edu") && !e.endsWith(".gov") && !e.endsWith(".net")))

Context

StackExchange Code Review Q#148775, answer score: 2

Revisions (0)

No revisions yet.