patternjavaModerate
Small password generator app with GUI
Viewed 0 times
appwithgeneratorpasswordsmallgui
Problem
This is a GUI-based random password generator, which supports internationalization.
It consists of 6 small classes:
Main classes:
Test classes:
Plus internationalization resource bundles which are not included here.
Please review on generally accepted rules / conventions / best practices and how can it be improved.
This is what the app looks like on Windows:
Application.java
```
package com.singularityfx.passgen;
import java.awt.EventQueue;
import java.io.IOException;
import java.io.InputStream;
import java.util.Locale;
import java.util.Properties;
import java.util.ResourceBundle;
import java.util.logging.Logger;
import javax.swing.UIManager;
import javax.swing.UIManager.LookAndFeelInfo;
import com.singularityfx.utils.ExceptionReporter;
/**
* Application entry point.
* Responsible for configuration of locale and look & feel, and for loading
* the GUI.
* @author SingularityFX
*
*/
public class Application {
private static Logger log = Logger.getLogger(Application.class.getName());
/**
* GUI messages for locale specified in resources/Locale.properties
*/
static ResourceBundle messages;
/**
* Loads GUI messages for locale specified in
* resources/Locale.properties from
* resources/MessagesBundle_<language>_<country>.properties
*/
private static void loadMessages() {
Properties localeProp = new Properties();
InputStream in = Application.class.getResourceAsStream("/Locale.properties");
try {
localeProp.load(in);
} catch (IOException e) {
log.severe(e.toString());
It consists of 6 small classes:
Main classes:
Application- entry point
GUI- self-explanatory
PassGen- class responsible for generating the password
ExceptionReporter- utility class for displaying the critical error messages to the user
Test classes:
MessagesBundleTest- tests internationalization resource bundles for completeness
PassGenTest- ensures that alphabet used for password generation is correct
Plus internationalization resource bundles which are not included here.
Please review on generally accepted rules / conventions / best practices and how can it be improved.
This is what the app looks like on Windows:
Application.java
```
package com.singularityfx.passgen;
import java.awt.EventQueue;
import java.io.IOException;
import java.io.InputStream;
import java.util.Locale;
import java.util.Properties;
import java.util.ResourceBundle;
import java.util.logging.Logger;
import javax.swing.UIManager;
import javax.swing.UIManager.LookAndFeelInfo;
import com.singularityfx.utils.ExceptionReporter;
/**
* Application entry point.
* Responsible for configuration of locale and look & feel, and for loading
* the GUI.
* @author SingularityFX
*
*/
public class Application {
private static Logger log = Logger.getLogger(Application.class.getName());
/**
* GUI messages for locale specified in resources/Locale.properties
*/
static ResourceBundle messages;
/**
* Loads GUI messages for locale specified in
* resources/Locale.properties from
* resources/MessagesBundle_<language>_<country>.properties
*/
private static void loadMessages() {
Properties localeProp = new Properties();
InputStream in = Application.class.getResourceAsStream("/Locale.properties");
try {
localeProp.load(in);
} catch (IOException e) {
log.severe(e.toString());
Solution
char[] pw = pg.generate(numberOfChars);
for (int i = 0; i < pw.length; i++) {
txtPW.append(Character.toString(pw[i]));
pw[i] = 0;
}What the...
Have you seen the
String(char[] value) constructor? There's also String.valueOf(char[] data). Those would make your loop irrelevant... unless there's a specific reason you're setting the pw array to 0?public GUI() {
// Set up panel
this.setLayout(new BoxLayout(this, BoxLayout.PAGE_AXIS));
this.setBorder(BorderFactory.createEmptyBorder(
BORDER_WIDTH, BORDER_WIDTH, BORDER_WIDTH, BORDER_WIDTH));
// Set up lblNumberOfChars and add it to panel
lblNumberOfChars.setAlignmentX(CENTER_ALIGNMENT);
this.add(lblNumberOfChars);
// Set up txtNumberOfChars and add it to panel
txtNumberOfChars.setAlignmentX(CENTER_ALIGNMENT);
txtNumberOfChars.setHorizontalAlignment(JTextField.LEFT);
this.add(txtNumberOfChars);
// Add empty space
this.add(Box.createRigidArea(new Dimension(0, BORDER_WIDTH)));
// Set up lblPW and add it to panel
lblPW.setAlignmentX(CENTER_ALIGNMENT);
this.add(lblPW);
// Set up txtPW and add it to panel
txtPW.setAlignmentX(CENTER_ALIGNMENT);
this.add(txtPW);
// Add empty space
this.add(Box.createRigidArea(new Dimension(0, BORDER_WIDTH)));
// Set up btnGenerate and add it to panel
btnGenerate.setAlignmentX(CENTER_ALIGNMENT);
btnGenerate.addActionListener(this);
this.add(btnGenerate);
}This function could make use of some meaningful whitespace.
public GUI() {
// Set up panel
this.setLayout(new BoxLayout(this, BoxLayout.PAGE_AXIS));
this.setBorder(BorderFactory.createEmptyBorder(
BORDER_WIDTH, BORDER_WIDTH, BORDER_WIDTH, BORDER_WIDTH));
// Set up lblNumberOfChars and add it to panel
lblNumberOfChars.setAlignmentX(CENTER_ALIGNMENT);
this.add(lblNumberOfChars);
// Set up txtNumberOfChars and add it to panel
txtNumberOfChars.setAlignmentX(CENTER_ALIGNMENT);
txtNumberOfChars.setHorizontalAlignment(JTextField.LEFT);
this.add(txtNumberOfChars);
// Add empty space
this.add(Box.createRigidArea(new Dimension(0, BORDER_WIDTH)));
// Set up lblPW and add it to panel
lblPW.setAlignmentX(CENTER_ALIGNMENT);
this.add(lblPW);
// Set up txtPW and add it to panel
txtPW.setAlignmentX(CENTER_ALIGNMENT);
this.add(txtPW);
// Add empty space
this.add(Box.createRigidArea(new Dimension(0, BORDER_WIDTH)));
// Set up btnGenerate and add it to panel
btnGenerate.setAlignmentX(CENTER_ALIGNMENT);
btnGenerate.addActionListener(this);
this.add(btnGenerate);
}Next, transform comments into code if they're explaining what the code does.
Specifically, this bit:
// Add empty space
this.add(Box.createRigidArea(new Dimension(0, BORDER_WIDTH)));That should be a function
addEmptySpaceToGUI().And the rest? Well, that could be functions too.
public GUI() {
setupPanel();
setupNumberOfCharactersField();
setupPasswordField();
setupGeneratePasswordButton();
}setupPanel() handles // Set up panel
this.setLayout(new BoxLayout(this, BoxLayout.PAGE_AXIS));
this.setBorder(BorderFactory.createEmptyBorder(
BORDER_WIDTH, BORDER_WIDTH, BORDER_WIDTH, BORDER_WIDTH));setupNumberOfCharactersField() handles// Set up lblNumberOfChars and add it to panel
lblNumberOfChars.setAlignmentX(CENTER_ALIGNMENT);
this.add(lblNumberOfChars);
// Set up txtNumberOfChars and add it to panel
txtNumberOfChars.setAlignmentX(CENTER_ALIGNMENT);
txtNumberOfChars.setHorizontalAlignment(JTextField.LEFT);
this.add(txtNumberOfChars);
// Add empty space
this.add(Box.createRigidArea(new Dimension(0, BORDER_WIDTH)));setupPasswordField() handles// Set up lblPW and add it to panel
lblPW.setAlignmentX(CENTER_ALIGNMENT);
this.add(lblPW);
// Set up txtPW and add it to panel
txtPW.setAlignmentX(CENTER_ALIGNMENT);
this.add(txtPW);
// Add empty space
this.add(Box.createRigidArea(new Dimension(0, BORDER_WIDTH)));and
setupGeneratePasswordButton() handles // Set up btnGenerate and add it to panel
btnGenerate.setAlignmentX(CENTER_ALIGNMENT);
btnGenerate.addActionListener(this);
this.add(btnGenerate);This way, you keep all the specifics out of your main functions. Maybe the names aren't even right, and it's more like
public GUI() {
setupPanel();
addNumberOfCharactersField();
addPasswordField();
addGeneratePasswordButton();
}This instead. With
add instead of setup we imply some ordering.Code Snippets
char[] pw = pg.generate(numberOfChars);
for (int i = 0; i < pw.length; i++) {
txtPW.append(Character.toString(pw[i]));
pw[i] = 0;
}public GUI() {
// Set up panel
this.setLayout(new BoxLayout(this, BoxLayout.PAGE_AXIS));
this.setBorder(BorderFactory.createEmptyBorder(
BORDER_WIDTH, BORDER_WIDTH, BORDER_WIDTH, BORDER_WIDTH));
// Set up lblNumberOfChars and add it to panel
lblNumberOfChars.setAlignmentX(CENTER_ALIGNMENT);
this.add(lblNumberOfChars);
// Set up txtNumberOfChars and add it to panel
txtNumberOfChars.setAlignmentX(CENTER_ALIGNMENT);
txtNumberOfChars.setHorizontalAlignment(JTextField.LEFT);
this.add(txtNumberOfChars);
// Add empty space
this.add(Box.createRigidArea(new Dimension(0, BORDER_WIDTH)));
// Set up lblPW and add it to panel
lblPW.setAlignmentX(CENTER_ALIGNMENT);
this.add(lblPW);
// Set up txtPW and add it to panel
txtPW.setAlignmentX(CENTER_ALIGNMENT);
this.add(txtPW);
// Add empty space
this.add(Box.createRigidArea(new Dimension(0, BORDER_WIDTH)));
// Set up btnGenerate and add it to panel
btnGenerate.setAlignmentX(CENTER_ALIGNMENT);
btnGenerate.addActionListener(this);
this.add(btnGenerate);
}public GUI() {
// Set up panel
this.setLayout(new BoxLayout(this, BoxLayout.PAGE_AXIS));
this.setBorder(BorderFactory.createEmptyBorder(
BORDER_WIDTH, BORDER_WIDTH, BORDER_WIDTH, BORDER_WIDTH));
// Set up lblNumberOfChars and add it to panel
lblNumberOfChars.setAlignmentX(CENTER_ALIGNMENT);
this.add(lblNumberOfChars);
// Set up txtNumberOfChars and add it to panel
txtNumberOfChars.setAlignmentX(CENTER_ALIGNMENT);
txtNumberOfChars.setHorizontalAlignment(JTextField.LEFT);
this.add(txtNumberOfChars);
// Add empty space
this.add(Box.createRigidArea(new Dimension(0, BORDER_WIDTH)));
// Set up lblPW and add it to panel
lblPW.setAlignmentX(CENTER_ALIGNMENT);
this.add(lblPW);
// Set up txtPW and add it to panel
txtPW.setAlignmentX(CENTER_ALIGNMENT);
this.add(txtPW);
// Add empty space
this.add(Box.createRigidArea(new Dimension(0, BORDER_WIDTH)));
// Set up btnGenerate and add it to panel
btnGenerate.setAlignmentX(CENTER_ALIGNMENT);
btnGenerate.addActionListener(this);
this.add(btnGenerate);
}// Add empty space
this.add(Box.createRigidArea(new Dimension(0, BORDER_WIDTH)));public GUI() {
setupPanel();
setupNumberOfCharactersField();
setupPasswordField();
setupGeneratePasswordButton();
}Context
StackExchange Code Review Q#66986, answer score: 10
Revisions (0)
No revisions yet.