A S.I.N checker, using Luhn's Algorithm, and created in swing.

I have created a program to validate a Canadian S.I.N. using Luhn's Algorithm, and was wondering if there was any way to make it more efficient. From what i can tell, it is already extremely efficient, but any tips or advice will be greatly appreciated.

```
import javax.swing.*;
import java.awt.*;
import java.awt.event.*;

class SinChecker extends JFrame
{ //naming variables
JTextField t1;
private JLabel j, j1, j2, j3;
ButtonListener bl1;
ButtonListener2 bl2;

public SinChecker ()
{ //Get the container
Container c = getContentPane ();

//Set absolute layout
c.setLayout (null);

//Set Background Color
c.setBackground (Color.WHITE);

//Creating label Guess my number text
JLabel j = new JLabel ("S.I.N. Validation Checker");
j.setForeground (Color.BLUE);
j.setFont (new Font ("tunga", Font.BOLD, 24));
j.setSize (270, 20);
j.setLocation (30, 35);

//Creating label Enter a number.....
j1 = new JLabel ("Enter your S.I.N. below.");
j1.setFont (new Font ("tunga", Font.PLAIN, 17));
j1.setSize (270, 20);
j1.setLocation (66, 60);

//Creating a label Instuctions
JLabel j2 = new JLabel ("Enter a 9-digit Social Insurance Number");
j2.setFont (new Font ("tunga", Font.PLAIN, 17));
j2.setSize (270, 20);
j2.setLocation (10, 165);

//Creating a label Instuctions
JLabel j3 = new JLabel ("with no spaces between the digits please.");
j3.setFont (new Font ("tunga", Font.PLAIN, 17));
j3.setSize (270, 20);
j3.setLocation (10, 180);

//Creating TextField for x input guess
t1 = new JTextField (10);
t1.setSize (70, 30);
t1.setLocation (100, 80);

//creating 2 buttons
JButton b1 = new JButton ("Proceed");
b1.setSize (120, 30);
b1.setLocation (70, 200);
bl1 = new ButtonListener ();
b1.addActionListener (bl1);

JButton b2 = new JButton ("Re-enter");
b2.setSize (120, 30);
b2.setLocation (70, 250);
bl2 = new ButtonListener2 ();
b2.addActionListener (bl2);

//Place the components in the pane
c.add (j);
c.add (j1);
c.add (j2);
c.add (j3);

c.add (t1);
c.add (b1);
c.add (b2);

//Set the title of the wi

User interface

For portability, you should use a layout manager, not absolutely positioned elements. Here's what it looks like on OS X:

I don't see a reason for two sets of instructions; the instructional text should be placed together.

"Validation Checker" is redundant. It's either a "SIN validator" or "SIN checker".

"Valid SIN" could be construed as meaning a number that has been actually been issued by Service Canada to a person. Perhaps you should clarify that it's merely a "possibly valid SIN".

Why prohibit spaces? Canadian Social Insurance Numbers are conventionally rendered as three groups of three digits, with spaces or hyphens between groups. Your program could easily be made more lenient and human-friendly, accepting and ignoring the superfluous punctuation.

To facilitate data entry, you should allow number keypad-only usage and perform the validation when Enter is pressed. (Currently, you have to press Tab and Space, or click Proceed.) For that matter, you could automatically perform the validation as soon as nine digits have been entered, without even requiring Enter.

Automatically returning focus to the number field and selecting its contents after validating was a nice touch.

Validation quality

"0" is accepted as a valid SIN.

"-9" is accepted as a valid SIN.

"123456789012345" or "hello" cause a NumberFormatException which is invisible in the UI. It just looks like either a valid SIN or an invalid SIN, depending on whether the previous validation passed or failed.

Implementation

You should implement the Luhn algorithm check in its own function (if not its own class) rather than in the event handler. That lets you reuse the code in other contexts such as a text-based UI, a tax-preparation program, or a unit test.

These lines of code are not as clear as they could be:

bl1 = new ButtonListener ();
b1.addActionListener (bl1);

Rather, I would prefer to see the button and text field identified with descriptive variable names. The ActionListener should be simplified and could be made into an anonymous inner class:

proceedButton.addActionListener(new ActionListener() {
    public void actionPerformed(ActionEvent e) {
        boolean valid = SinChecker.isValid(sinTextField.getText());
        SinChecker.this.announceValid(valid);
        sinTextField.requestFocus();
        sinTextField.selectAll();
    }
});

Since your SinChecker class extends JFrame, I would expect its constructor to behave a specialized JFrame. In particular, a JFrame constructor would not make the newly instantiated frame visible, nor would it set exit-on-close. Therefore, your SinChecker constructor shouldn't do those things, either. If your SinChecker is incorporated as a component within some larger program, exit-on-close might not be appropriate.

Speaking of reusability, consider making your SinChecker a JPanel instead of a JFrame, so that you have the flexibility to use it as a widget that is not its own window. In that case, your widget should offer a getSin() method that returns a guaranteed valid SIN.

In accordance with the documentation, Swing code must run in the Event Dispatch Thread, not the main thread.

public class SinChecker extends JPanel {
    public SinChecker() {
        this.setBackground(Color.WHITE);
        …
        this.setPreferredSize(…);  // Ideally automatically determined by layout manager
    }

    …

    public static void main(String[] args) {
        SwingUtilities.invokeLater(new Runnable() {
            public void run() {
                JFrame f = new JFrame("Social Insurance Number Checker");
                f.setLayout(new BorderLayout());
                f.add(new SinChecker());
                f.pack();
                f.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
                f.setVisible(true);
            }
        });
    }
}

StackExchange Code Review Q#87864, answer score: 4