###EditorMain
EditorMain
###ColorTableModel
ColorTableModel
###EditorMain
###ColorTableModel
EditorMain
ColorTableModel
Note that the TableModel import is not on the list of things I said to replace. It's unused, so you should get rid of it. Ditto for java.io.FileNotFoundException.
Nothing to criticize here. Just wanted to give you an internet pat-on-the-back for doing this. It's a very good idea to separate graphics and processing.
for (UIManager.LookAndFeelInfo info : UIManager.getInstalledLookAndFeels()) {
if ("Nimbus".equals(info.getName())) {
UIManager.setLookAndFeel(info.getClassName());
break;
}
}
This took me a couple of seconds to get and a read through the docs -- it might be worth it to add a comment along the lines of
// Use the theme called "Nimbus" if it's installed.
###ColorTableModeljava.util.Enumeration is a raw type; it takes a generic, so you should use it with one. In this case, it'd just be java.util.Enumeration<Object>, but it'll help prevent errors later on if you add the generic now.
I'd recommend adding some sort of check inIn getopenMenuItem.addActionListener and, you never actually used setValueAtfile. I'm assuming that's for later, but I wanted to point it out anyway; I'd add a System.out.println(Object, int, intfile.getAbsolutePath()); so you can see that it's the right file.
###ColorTableModel
I'd recommend adding a check wherever you access an array with a value from a parameter. While it's not strictly necessary, since trying to call anything out-of-bounds will properly throw an error, getting an ArrayIndexOutOfBounds when it doesn't look like you're accessing an array can be confusing. It's as simple as wrapping it in a try/catch and throwing an IllegalArgumentException if you get an ArrayIndexOutOfBounds -- i.e. something like this:
// Same idea for bothall of them, so I'm just using `get` as an example.
public Object getValueAt(int row, int column) {
try {
rowData[row][column];
} catch (ArrayIndexOutOfBoundsException e) {
throw new IllegalArgumentException(String.format(Locale.ENGLISH, "(%s, %s) is an invalid location!", row, column), e);
}
}
I used String#format because I personally find it nicer to use. You can use whatever you likeConcatenation would work equally well.
public Class getColumnClass(int column) {
return (getValueAt(0, column).getClass());
}
Going to go eat. I'll take another look once that's done and add more if I see anything. However, it looks good to me! This should return a Class<?> to conform with AbstractTableModel.
Note that the TableModel import is not on the list of things I said to replace.
Nothing to criticize here. Just wanted to give you an internet pat-on-the-back for doing this.
###ColorTableModel
I'd recommend adding some sort of check in get and setValueAt(Object, int, int). While it's not strictly necessary, since trying to call anything out-of-bounds will properly throw an error, getting an ArrayIndexOutOfBounds when it doesn't look like you're accessing an array can be confusing. It's as simple as wrapping it in a try/catch and throwing an IllegalArgumentException if you get an ArrayIndexOutOfBounds -- i.e. something like this:
// Same idea for both, so I'm just using `get` as an example.
public Object getValueAt(int row, int column) {
try {
rowData[row][column];
} catch (ArrayIndexOutOfBoundsException e) {
throw new IllegalArgumentException(String.format(Locale.ENGLISH, "(%s, %s) is an invalid location!", row, column), e);
}
}
I used String#format because I personally find it nicer to use. You can use whatever you like.
Going to go eat. I'll take another look once that's done and add more if I see anything. However, it looks good to me!
Note that the TableModel import is not on the list of things I said to replace. It's unused, so you should get rid of it. Ditto for java.io.FileNotFoundException.
Nothing to criticize here. Just wanted to give you an internet pat-on-the-back for doing this. It's a very good idea to separate graphics and processing.
for (UIManager.LookAndFeelInfo info : UIManager.getInstalledLookAndFeels()) {
if ("Nimbus".equals(info.getName())) {
UIManager.setLookAndFeel(info.getClassName());
break;
}
}
This took me a couple of seconds to get and a read through the docs -- it might be worth it to add a comment along the lines of
// Use the theme called "Nimbus" if it's installed.
java.util.Enumeration is a raw type; it takes a generic, so you should use it with one. In this case, it'd just be java.util.Enumeration<Object>, but it'll help prevent errors later on if you add the generic now.
In openMenuItem.addActionListener, you never actually used file. I'm assuming that's for later, but I wanted to point it out anyway; I'd add a System.out.println(file.getAbsolutePath()); so you can see that it's the right file.
###ColorTableModel
I'd recommend adding a check wherever you access an array with a value from a parameter. While it's not strictly necessary, since trying to call anything out-of-bounds will properly throw an error, getting an ArrayIndexOutOfBounds when it doesn't look like you're accessing an array can be confusing. It's as simple as wrapping it in a try/catch and throwing an IllegalArgumentException if you get an ArrayIndexOutOfBounds -- i.e. something like this:
// Same idea for all of them, so I'm just using `get` as an example.
public Object getValueAt(int row, int column) {
try {
rowData[row][column];
} catch (ArrayIndexOutOfBoundsException e) {
throw new IllegalArgumentException(String.format(Locale.ENGLISH, "(%s, %s) is an invalid location!", row, column), e);
}
}
I used String#format because I personally find it nicer to use. Concatenation would work equally well.
public Class getColumnClass(int column) {
return (getValueAt(0, column).getClass());
}
This should return a Class<?> to conform with AbstractTableModel.
###EditorMain
import javax.swing.ImageIcon;
import javax.swing.JButton;
import javax.swing.JComponent;
// ...You get the idea.
import javax.swing.UIManager;
import javax.swing.SwingUtilities;
import javax.swing.WindowConstants;
At this point, you may as well just do the one * import statement:
import javax.swing.*;
Note that the TableModel import is not on the list of things I said to replace.
EditorMain() {
This should probably be public; you very rarely need to use package-private, and this doesn't seem like one of those cases. Especially since the class is public and contains main(String[] args).
You never seem to add click listeners to cut, copy, and pasteMenuItem. I suppose this is because you're in a basic version of the code, but at the very least, add a placeholder comment like
//TODO: Add listeners to these
between openMenuItem and the other three. It'll keep you from forgetting.
JScrollPane jscroll = new JScrollPane(table);
jscroll is a terrible name. Admittedly, there's really not a good name for a JScrollPane wrapper, but what I'd do is something like tableScroller, so it's at least clear what it's wrapped around.
} catch (Exception e) {
This is very often terrible idea. Always catch the most specific exceptions you can, with as many catch blocks as is reasonable -- in this case, you'll want to refer to the UIManager#setLookAndFeel Javadocs. They tend to list the exceptions (at least in my experience) in the same order that you should have list them when writing your try/catch blocks.
javax.swing.plaf.FontUIResource
You use the fully qualified name at least twice. Why not import it at the top?
SwingUtilities.invokeLater(EditorMain::new);
Nothing to criticize here. Just wanted to give you an internet pat-on-the-back for doing this.
###ColorTableModel
I'd recommend adding some sort of check in get and setValueAt(Object, int, int). While it's not strictly necessary, since trying to call anything out-of-bounds will properly throw an error, getting an ArrayIndexOutOfBounds when it doesn't look like you're accessing an array can be confusing. It's as simple as wrapping it in a try/catch and throwing an IllegalArgumentException if you get an ArrayIndexOutOfBounds -- i.e. something like this:
// Same idea for both, so I'm just using `get` as an example.
public Object getValueAt(int row, int column) {
try {
rowData[row][column];
} catch (ArrayIndexOutOfBoundsException e) {
throw new IllegalArgumentException(String.format(Locale.ENGLISH, "(%s, %s) is an invalid location!", row, column), e);
}
}
I used String#format because I personally find it nicer to use. You can use whatever you like.
Going to go eat. I'll take another look once that's done and add more if I see anything. However, it looks good to me!