Class for serializing themes for my JavaFX application

Question

I have a Theme class for storing themes for my javaFX application. I created a serialize method that stores its information as a String and a constructor to create a Theme from a serialized Theme. I had to do this because I can't make Theme implement Serializable, because its fields can't, and I need to store Theme objects after the program ends.

The code works fine, but it was tedious to write. I'm wondering if it's needlessly complicated or really inefficient, because it seems like there would be a native fix for this.

import javafx.scene.paint.Color;

public class Theme {
    private Color btnBackground; 
    private Color btnBorder; 
    private Color labelFill; 
    private Color panesBackground; 
    private Color txtFieldBackground; 
    private Color txtFieldBorder; 
    private String btnStyle; 
    private String txtFieldStyle; 
    private String name; 

    public Theme(Color panesBackground, Color labelFill, Color txtFieldBackground, Color txtFieldBorder, String txtFieldStyle, Color btnBackground, Color btnBorder, String btnStyle, String name) {
        this.panesBackground = panesBackground;
        this.labelFill = labelFill;
        this.txtFieldBackground = txtFieldBackground;
        this.txtFieldBorder = txtFieldBorder;
        this.txtFieldStyle = txtFieldStyle;
        this.btnBackground = btnBackground;
        this.btnBorder = btnBorder;
        this.btnStyle = btnStyle;
        this.name = name;
    }

    public Theme(String serializedTheme) {
        String[] themeInfo = serializedTheme.split("#");
        String[] colors = themeInfo[0].split("@");
        String[][] strColorVals = new String[6][3];
        for(int i = 0; i < 6; i++) {
            strColorVals[i] = colors[i].split(",");
        }
        float[][] colorVals = new float[6][3];
        for(int i = 0; i < colorVals.length; i++) {
            for(int j = 0; j < colorVals[i].length; j++) {
                colorVals[i][j] = Float.parseFloat(strColorVals[i][j]);
            }
        }
        String[] strings = themeInfo[1].split("@");
        panesBackground = Color.color(colorVals[3][0], colorVals[3][1], colorVals[3][2]);
        labelFill = Color.color(colorVals[2][0], colorVals[2][1], colorVals[2][2]);
        txtFieldBackground = Color.color(colorVals[4][0], colorVals[4][1], colorVals[4][2]);
        txtFieldBorder = Color.color(colorVals[5][0], colorVals[5][1], colorVals[5][2]);
        txtFieldStyle = strings[1];
        btnBackground = Color.color(colorVals[0][0], colorVals[0][1], colorVals[0][2]);
        btnBorder = Color.color(colorVals[1][0], colorVals[1][1], colorVals[1][2]);
        btnStyle = strings[0];
        name = strings[2];
    }

    public static float[] colorToArr(Color color) {
        return new float[]{(float)color.getRed(), (float)color.getGreen(), (float)color.getBlue()};
    }

    public Color getBtnBorder() {
        return btnBorder;
    }

    public Color getBtnBackground() {
        return btnBackground;
    }

    public Color getTxtFieldBorder() {
        return txtFieldBorder;
    }

    public Color getTxtFieldBackground() {
        return txtFieldBackground;
    }

    public Color getLabelFill() {
        return labelFill;
    }

    public Color getPanesBackground() {
        return panesBackground;
    }

    public String getBtnStyle() {
        return btnStyle;
    }

    public String getTxtFieldStyle() {
        return txtFieldStyle;
    }

    public String getName() {
        return name;
    }

    public String serialize() {
        return colorToArr(btnBackground)[0] + "," +
        colorToArr(btnBackground)[1] + "," +
        colorToArr(btnBackground)[2] + "@" +
        colorToArr(btnBorder)[0] + "," +
        colorToArr(btnBorder)[1] + "," +
        colorToArr(btnBorder)[2] + "@" +
        colorToArr(labelFill)[0] + "," +
        colorToArr(labelFill)[1] + "," +
        colorToArr(labelFill)[2] + "@" +
        colorToArr(panesBackground)[0] + "," +
        colorToArr(panesBackground)[1] + "," +
        colorToArr(panesBackground)[2] + "@" +
        colorToArr(txtFieldBackground)[0] + "," +
        colorToArr(txtFieldBackground)[1] + "," +
        colorToArr(txtFieldBackground)[2] + "@" +
        colorToArr(txtFieldBorder)[0] + "," +
        colorToArr(txtFieldBorder)[1] + "," +
        colorToArr(txtFieldBorder)[2] + "#" +
        btnStyle + "@" + txtFieldStyle + "@" + name;
    }

}

I have a method in my main class for reading the file to a list of Theme's:

// create themes file or update themes list
        if (themesFile.createNewFile()) {
            themesList.add(darkTheme);
            themesList.add(lightTheme);
            try {
                // write theme information to file:
                FileWriter wrtr = new FileWriter(themesFile);
                for(Theme theme : themesList) {
                    wrtr.append(theme.serialize());
                }
                wrtr.close();
                System.out.println("Themes file created.");
            } catch (IOException ex) {
                ex.printStackTrace();
            }
        }else {
            System.out.println("Themes file already exists.");
            Scanner scnr = new Scanner(themesFile);
            while(scnr.hasNext()) {
                themesList.add(new Theme(scnr.next()));
            }
            scnr.close();
        }

Note: themesList is an ArrayList of themes, and Light and Dark are theme fields of the main class. Their serialize() methods return

"0.2,0.2,[email protected],0.7,[email protected],0.8,[email protected],0.2,[email protected],0.3,[email protected],0.7,0.7#-fx-text-fill:lightgrey@-fx-text-fill:white@Dark"

and

"0.9,0.9,[email protected],0.5,[email protected],0.0,[email protected],1.0,[email protected],1.0,[email protected],0.5,0.5#-fx-text-fill:black@-fx-text-fill:black@Light"

respectively.

Answer 1

Here at Code Review we're not normally meant to simply suggest alternative solutions, but in this case, you already mentioned the alternative and I'm really just suggesting that you look again at your reason for not using this Serializable interface and the related tools (like ObjectOutputStream). They are made for this purpose -- and they do have ways to handle fields that are not themselves Serializable.

Here's how that would look.

import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.io.Serializable;

import javafx.scene.paint.Color;

public class Theme implements Serializable {
    private static final long serialVersionUID = 1L;

    private Color btnBackground;
    private Color btnBorder;
    private Color labelFill;
    private Color panesBackground;
    private Color txtFieldBackground;
    private Color txtFieldBorder;
    private String btnStyle;
    private String txtFieldStyle;
    private String name;

    public Theme(Color btnBackground, Color btnBorder, Color labelFill,
            Color panesBackground, Color txtFieldBackground, Color txtFieldBorder,
            String btnStyle, String txtFieldStyle, String name) {
        this.btnBackground = btnBackground;
        this.btnBorder = btnBorder;
        this.labelFill = labelFill;
        this.panesBackground = panesBackground;
        this.txtFieldBackground = txtFieldBackground;
        this.txtFieldBorder = txtFieldBorder;
        this.btnStyle = btnStyle;
        this.txtFieldStyle = txtFieldStyle;
        this.name = name;
    }

    private static void writeColor(ObjectOutputStream out, Color color)
            throws IOException {
        out.writeDouble(color.getRed());
        out.writeDouble(color.getGreen());
        out.writeDouble(color.getBlue());
    }

    private static Color readColor(ObjectInputStream in) throws IOException {
        return Color.color(in.readDouble(), in.readDouble(), in.readDouble());
    }

    private void writeObject(ObjectOutputStream out) throws IOException {
        writeColor(out, btnBackground);
        writeColor(out, btnBorder);
        writeColor(out, labelFill);
        writeColor(out, panesBackground);
        writeColor(out, txtFieldBackground);
        writeColor(out, txtFieldBorder);
        out.writeObject(btnStyle);
        out.writeObject(txtFieldStyle);
        out.writeObject(name);
    }

    private void readObject(ObjectInputStream in)
            throws IOException, ClassNotFoundException {
        this.btnBackground = readColor(in);
        this.btnBorder = readColor(in);
        this.labelFill = readColor(in);
        this.panesBackground = readColor(in);
        this.txtFieldBackground = readColor(in);
        this.txtFieldBorder = readColor(in);
        this.btnStyle = (String) in.readObject();
        this.txtFieldStyle = (String) in.readObject();
        this.name = (String) in.readObject();
    }
}

There's nothing fancy about it, just writing the fields in the same order they appear in the class and reading them back in the same order. You don't need to worry about delimiters with this.

And here's the example of how to use it. This serializes it into a byte[] and then pulls it out again but you could replace ByteArrayOutputStream with FileOutputStream or whatever.

// import java.io.ByteArrayInputStream;
// import java.io.ByteArrayOutputStream;

    public static void main(String[] args) throws IOException, ClassNotFoundException {
        Theme light = new Theme(Color.WHITE, Color.WHITE, Color.WHITE, Color.WHITE,
                Color.WHITE, Color.WHITE, "-fx-text-fill:lightgrey",
                "-fx-text-fill:white", "Light");

        byte[] serialized;
        try (ByteArrayOutputStream buffer = new ByteArrayOutputStream();
                ObjectOutputStream out = new ObjectOutputStream(buffer)) {
            out.writeObject(light);
            serialized = buffer.toByteArray();
        }

        Theme light2;
        try (ObjectInputStream in = new ObjectInputStream(
                new ByteArrayInputStream(serialized))) {
            light2 = (Theme) in.readObject();
        }

        System.out.println(light2.btnBackground);
        System.out.println(light2.name);
    }

Answer 2

You definitely have inefficiency. Consider:

        ...                                 +
        colorToArr(btnBorder)[0] + "," +
        colorToArr(btnBorder)[1] + "," +
        colorToArr(btnBorder)[2] + "@" +
        ...

Here, colorToArr(...) is being called 3 times with the same argument, and it has to allocate, compute, and return the same value each time, only to have one subcomponent of the returned value extracted from the aggregate value.

Not only is this inefficient for the program, it was inefficient for the programmer to type!

If instead you created a colorToString(...) function:

    private static String colorToString(Color color) {
        return (float)color.getRed() + "," + (float)color.getGreen() + "," + (float)color.getBlue();
    }

Now you could write your serialize() as:

    public String serialize() {
        return colorToString(btnBackground) + "@" +
               colorToString(btnBorder) + "@" +
               colorToString(labelFill) + "@" +
               colorToString(panesBackground) + "@" +
               colorToString(txtFieldBackground) + "@" +
               colorToString(txtFieldBorder) + "#" +
               btnStyle + "@" + txtFieldStyle + "@" + name;
    }

However, creating & returning many small strings can also be inefficient, so a StringBuilder might be appropriate to use here.

    private static void colorToString(StringBuilder sb, Color color, char end) {
        sb.append((float)color.getRed().append(',')
          .append((float)color.getGreen().append(',')
          .append((float)color.getBlue().append(end);
    }

    public String serialize() {
        StringBuilder sb = new StringBuilder(100);

        colorToString(sb, btnBackground, '@');
        colorToString(sb, btnBorder, '@');
        colorToString(sb, labelFill, '@');
        colorToString(sb, panesBackground, '@');
        colorToString(sb, txtFieldBackground, '@');
        colorToString(sb, txtFieldBorder, '#');
        sb.append(btnStyle).append('@')
          .append(txtFieldStyle).append('@')
          .append(name);

        return sb.toString();
    }

Moving on ...

        panesBackground = Color.color(colorVals[3][0], colorVals[3][1], colorVals[3][2]);
        labelFill = Color.color(colorVals[2][0], colorVals[2][1], colorVals[2][2]);
        txtFieldBackground = Color.color(colorVals[4][0], colorVals[4][1], colorVals[4][2]);
        txtFieldBorder = Color.color(colorVals[5][0], colorVals[5][1], colorVals[5][2]);
        txtFieldStyle = strings[1];
        btnBackground = Color.color(colorVals[0][0], colorVals[0][1], colorVals[0][2]);
        btnBorder = Color.color(colorVals[1][0], colorVals[1][1], colorVals[1][2]);

Yuk. Lots of repetitive array indexing. A function would help. We can even move the splitting on commas and float parsing into the function, eliminating the allocation of the temporary strColVals and colorVals 2-d arrays:

    private static Color stringToColor(String color) {
        String vals[] = color.split(",");
        float red = Float.parseFloat(vals[0]);
        float green = Float.parseFloat(vals[1]);
        float blue = Float.parseFloat(vals[2]);
        return Color.color(red, green, blue);
    }

    public Theme(String serializedTheme) {
        String[] themeInfo = serializedTheme.split("#");
        String[] colors = themeInfo[0].split("@");
        ...
        btnBackground      = stringToColor(colors[0]);
        btnBorder          = stringToColor(colors[1]);
        labelFill          = stringToColor(colors[2]);
        panesBackground    = stringToColor(colors[3]);
        txtFieldBackground = stringToColor(colors[4]);
        txtFieldBorder     = stringToColor(colors[5]);
        ...
    }

With the colors extracted in ascending order -- instead of 3,2,4,5,0,1 -- we can see the same order of color name fields in the serialization & deserialization, making the code much more maintainable.