patternjavaMinor
Java Swing Application - Roleplaying Aid
Viewed 0 times
applicationjavaswingroleplayingaid
Problem
I've been working on this utility for a while now, and have since gone through a couple reworks to try and increase the readability and best practices, but since I don't usually work a lot with Swing, or UI design in general, I still have the nagging feeling there's a better way to approach this. Code for both files is below along with a GitHub link in case full context is needed.
Specific Questions
Additional Edit/Note: There is plans for additional UI/functions for this project, so code was brought outside of LifePathUI for reuse purposes as well. I just realized that would also be important context as to intent.
Main UI (LifePathUI)
```
import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JOptionPane;
import javax.swing.JScrollPane;
import javax.swing.JTextArea;
import javax.swing.event.DocumentEvent;
import javax.swing.event.DocumentListener;
public class LifePathUI implements UI {
final static boolean shouldWeightX = true;
final static String DIVIDER_STRING = "\n------------------------------------------\n";
// we still hardcode some stats like this because the page would break if they were user definable anyways
final static String[] primStats = {"COG","COO","INT","REF","SAV","SOM","WIL"};
final static String[] secStats = {"DUR","WT","DR","LUC","TT","IR","INIT","SPD",
Specific Questions
- Is
GridBagUIPanela proper abstraction of functionality, or is it too little/too much?
- Is there anything I should be doing to clean up init()/update() more in
LifePathUI?
- Am I using things right with the
TextChangeListeners and Button Events inLifePathUI? Defining classes inside method calls still feels very off to me.
- Is there any glaring reinventing the wheel or other mistakes going on? I want to cut out any particularly embarrassing things since this is one of my few projects that would be able to be used in portfolios and such.
Additional Edit/Note: There is plans for additional UI/functions for this project, so code was brought outside of LifePathUI for reuse purposes as well. I just realized that would also be important context as to intent.
Main UI (LifePathUI)
```
import java.awt.*;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JOptionPane;
import javax.swing.JScrollPane;
import javax.swing.JTextArea;
import javax.swing.event.DocumentEvent;
import javax.swing.event.DocumentListener;
public class LifePathUI implements UI {
final static boolean shouldWeightX = true;
final static String DIVIDER_STRING = "\n------------------------------------------\n";
// we still hardcode some stats like this because the page would break if they were user definable anyways
final static String[] primStats = {"COG","COO","INT","REF","SAV","SOM","WIL"};
final static String[] secStats = {"DUR","WT","DR","LUC","TT","IR","INIT","SPD",
Solution
Quite nice helper. Also, thanks from RPG fan, I'll check out Eclipse Phase later as well. :-)
My remarks:
No packages!
If the project is to be a portfolio one, use
Read on packages prior to choosing how to group your code.
Code quality, various remarks
I'm sorry for not being able to process and review code fully. Here are things that stood out for me.
I'm mentioning these only briefly as when one usually checks code for clarity, one uses tools. Tools report these, so you should be aware, if you want to use project to show your skills. Some, you may want to fix, depending on whom you want to impress.
Javadocs
-
The class Javadoc is often empty, save
-
Javadocs for getters and setters are sometimes poor (autogenerated, it seems). Please remove. They bring no value, functions are trivial one-liners with very common purpose and naming adhering to convention. That is enough, Javadocs for such things are only making the file bloated. Examples: Aptitude, Character.
-
If you have file comments that are empty, remove them. If you have file comments, that hold value to class (example: Function, Sleight, ExistsValidator) not to file, integrate them as Javadocs. Good file comment might be about encoding, or file being tool-generated. I'm yet to see one that's about code and it makes sense for it to be a file comment. For your convenience: File comment is a comment at the top of the file (or just under package and imports, but prior to class javadoc and class itself).
Working with Strings
There are two aspects here. One, this application is very String-reliant. Are you sure you wouldn't want to use types more? The other thing is technical aspect of using Strings.
Technically
You have a Utils class, that works with Strings heavily. Please take a look at projects like StringUtils from Apache (or other, similar). You may find them to your liking, they do a lot of nice String operations: join, split, isEmptyOrBlank, etc. I believe there's also a sister library, NumberUtils, which has a method to check if String contains a digit.
Of course, feel free to reject pulling outside libraries into your project - there may be good reasons not to. However:
Up to you here. Both libraries can be downloaded together if you pull in Apache Commons. There's also Google alternative: Guava (even better IMO).
More info:
https://stackoverflow.com/questions/1444437/apache-commons-vs-guava-formerly-google-collections
https://stackoverflow.com/questions/7015739/is-there-a-viable-generic-alternative-to-apache-commons-collections-collectionut
http://eclipsesource.com/blogs/2013/11/06/get-rid-of-your-stringutils/
Objects vs Strings
Consider Sleight class. In excerpt taken from it, everything is a String!
```
private String sleightType;
private String isExsurgent;
private String name;
private String psiType;
private String actionType;
private String range;
private String duration;
private String strainMod;
private String skillUsed;
private String description;
// stores all the below sleights
public static HashMap sleightList = new HashMap();
/**
* @param sleightType Classification of Sleight : chi, gamma, etc
* @param isExsurgent Whether the sleight is meant to be exsurgent only
* @param name Name of the sleight
* @param psiType Active or Passive
* @param actionType What type of action the sleight uses
* @param range Range of the sleight
* @param duration Duration of the sleight
* @param strainMo
My remarks:
No packages!
If the project is to be a portfolio one, use
package keyword and put your classes in packages. There is number of advantages:- packages organize areas or your program
- they put related classes together
- lack of packages means, that your code is all in ONE package, so-called default package. That's considered a code smell.
- so-called Fully Qualified Name of a class includes package. Therefore, I can ran number of classes called MyClass within 1 JVM, as long as they have different packages. With default package, there's greater risk of confusing JVM: which MyClass is which?
Read on packages prior to choosing how to group your code.
Code quality, various remarks
I'm sorry for not being able to process and review code fully. Here are things that stood out for me.
- Interfaces methods are public by default. You need not specify that.
- Each time you have an
if X then true else falsesimplify it toreturn X;(Skill, LifePathUI, CharacterSheetUI).
- If you
import a.b., there's no need toimport a.b.somethingadditionally, asa.b.already did so.
- In number of places you have methods, values, local variables and parameters that are never used. Consider cutting. IDE can show you these, as well as FindBugs or Checkstyle or SonarQube tools.
- I found three type casts that were redundant. All to int. All in Character class.
I'm mentioning these only briefly as when one usually checks code for clarity, one uses tools. Tools report these, so you should be aware, if you want to use project to show your skills. Some, you may want to fix, depending on whom you want to impress.
Javadocs
-
The class Javadoc is often empty, save
@author tag. Please explain the class in class Javadoc, number of IDEs show such description when you hover over object of X type. It's quite helpful for others. Examples: Aptitude, Character.-
Javadocs for getters and setters are sometimes poor (autogenerated, it seems). Please remove. They bring no value, functions are trivial one-liners with very common purpose and naming adhering to convention. That is enough, Javadocs for such things are only making the file bloated. Examples: Aptitude, Character.
-
If you have file comments that are empty, remove them. If you have file comments, that hold value to class (example: Function, Sleight, ExistsValidator) not to file, integrate them as Javadocs. Good file comment might be about encoding, or file being tool-generated. I'm yet to see one that's about code and it makes sense for it to be a file comment. For your convenience: File comment is a comment at the top of the file (or just under package and imports, but prior to class javadoc and class itself).
Working with Strings
There are two aspects here. One, this application is very String-reliant. Are you sure you wouldn't want to use types more? The other thing is technical aspect of using Strings.
Technically
You have a Utils class, that works with Strings heavily. Please take a look at projects like StringUtils from Apache (or other, similar). You may find them to your liking, they do a lot of nice String operations: join, split, isEmptyOrBlank, etc. I believe there's also a sister library, NumberUtils, which has a method to check if String contains a digit.
Of course, feel free to reject pulling outside libraries into your project - there may be good reasons not to. However:
NumberUtil.isNumber(String str)also factors in Unicode digits
- Knowing common libraries looks good in portfolio.
- You don't need to maintain code that doesn't add to your domain (Eclipse Phase RPG helper).
Up to you here. Both libraries can be downloaded together if you pull in Apache Commons. There's also Google alternative: Guava (even better IMO).
More info:
https://stackoverflow.com/questions/1444437/apache-commons-vs-guava-formerly-google-collections
https://stackoverflow.com/questions/7015739/is-there-a-viable-generic-alternative-to-apache-commons-collections-collectionut
http://eclipsesource.com/blogs/2013/11/06/get-rid-of-your-stringutils/
Objects vs Strings
Consider Sleight class. In excerpt taken from it, everything is a String!
```
private String sleightType;
private String isExsurgent;
private String name;
private String psiType;
private String actionType;
private String range;
private String duration;
private String strainMod;
private String skillUsed;
private String description;
// stores all the below sleights
public static HashMap sleightList = new HashMap();
/**
* @param sleightType Classification of Sleight : chi, gamma, etc
* @param isExsurgent Whether the sleight is meant to be exsurgent only
* @param name Name of the sleight
* @param psiType Active or Passive
* @param actionType What type of action the sleight uses
* @param range Range of the sleight
* @param duration Duration of the sleight
* @param strainMo
Code Snippets
private String sleightType;
private String isExsurgent;
private String name;
private String psiType;
private String actionType;
private String range;
private String duration;
private String strainMod;
private String skillUsed;
private String description;
// stores all the below sleights
public static HashMap<String,Sleight> sleightList = new HashMap<String,Sleight>();
/**
* @param sleightType Classification of Sleight : chi, gamma, etc
* @param isExsurgent Whether the sleight is meant to be exsurgent only
* @param name Name of the sleight
* @param psiType Active or Passive
* @param actionType What type of action the sleight uses
* @param range Range of the sleight
* @param duration Duration of the sleight
* @param strainMod Strain mod of the sleight (if any)
* @param skillUsed Skill the sleight uses (if active
* @param description Human readable description of sleightIs GridBagUIPanel a proper abstraction of functionality, or is it too little/too much?Is there anything I should be doing to clean up init()/update() more in LifePathUI?import javax.swing.*;
/**
* Created by LAFK on 01.09.15.
*/
public class MyJFrame extends JFrame {
private final MyJFrame frame;
public MyJFrame(JFrame frame) {
this.frame = (MyJFrame)frame;
}
public MyJFrame addStatPanel(JPanel panel) {
frame.add(panel);
return frame;
}
}Am I using things right with the TextChangeListeners and Button Events in LifePathUI? Defining classes inside method calls still feelsContext
StackExchange Code Review Q#102268, answer score: 3
Revisions (0)
No revisions yet.