4
\$\begingroup\$

I created simple export tool for my project. My requirments was to create one csv file for details of my hero which contains name, level, gold etc. And second file for list of items. Both of files contains static header and generated Data.

First, let me share my model classes:

public class Hero {
    private String name;
    private int level;
    private List<Item> items = new ArrayList();
    private String className;
    private BigDecimal gold;

//getters setters constructors
}


public class Item {

    private int id;
    private String name;
    private double weight;
//getters setters constructors
}



public class Weapon extends Item {

    private int dmg;
    private int level;
//getters setters constructors
}



public class Food extends Item {

    private int capacity;
    private int hpRegen;
//getters setters constructors
}

I wont share real classes, because they are a little too big, so I have prepared sample classes.

And here we go with example 'exporter'. This class has two public methods which return CSVWriter which we can use to save data to file.

public class HeroWriter {

    private Hero hero;
    
    private final static String[] HERO_INFO_HEADER = {
            "server",
            "date",
            "name",
            "class",
            "level",
            "gold"
    };
    
    private final static String[] ITEM_LIST_HEADER = {
            "id",
            "name",
            "weight",
            "dmg",
            "hp",
            "required level",
            "capacity"
    };
    
    public HeroWriter(Hero hero) {
        this.hero = hero;
    }
    
    
    public CSVWriter writeHeroInformation() {
        CSVWriter writer = new CSVWriter(null);
        writer.writeNext(HERO_INFO_HEADER);
        
        String[] heroInformation = {
                "Mocked Server",
                new Date().toString(),
                hero.getName(),
                hero.getClassName(),
                hero.getLevel()+"",
                hero.getGold().toString()
        };
        
        writer.writeNext(heroInformation);
        
        return writer;
    }
    
    public CSVWriter writeItemList() {
        CSVWriter writer = new CSVWriter(null);
        writer.writeNext(ITEM_LIST_HEADER);
        
        for (Item item : hero.getItems()) {
            if(item instanceof Weapon) {
                Weapon weapon = (Weapon) item;
                
                String[] weaponDetail = {
                        weapon.getId() + "",
                        weapon.getName(),
                        weapon.getWeight() + "",
                        weapon.getDmg() + "",
                        "-",
                        weapon.getLevel() + "",
                        "-"
                };
                writer.writeNext(weaponDetail);
                
            } else if (item instanceof Food) {
                Food food = (Food) item;
                
                String[] foodDetail = {
                        food.getId() + "",
                        food.getName(),
                        food.getWeight() + "",
                        "-",
                        food.getHpRegen() + "",
                        "-",
                        food.getCapacity() + ""
                };
                writer.writeNext(foodDetail);
            }
        }
        
        return writer;
    }   
}

I would like to as you to review HeroWriter class, because I have problem with OOP and would like to improve in this aspect.

I was thinking that maybe I could create Exportable interface which contains method like String[] export(); and I would implement it in Hero/Weapon/Item class that would help me remove instanceof part of code. But I am not sure if it is good idea.

\$\endgroup\$
1
  • \$\begingroup\$ Hello, have you implemented the toString method in your classes ? \$\endgroup\$ Sep 17, 2020 at 15:43

1 Answer 1

5
\$\begingroup\$
hero.getLevel()+""

Use an explicit cast instead of implicit one:

Integer.toString(hero.getLevel())

```java
new Date().toString(),
// ...
hero.getGold().toString()

Which might or might not be the format you want in the final file. Ideally you'd define explicit formatting rules.


There are a few approaches to serialization of data like in your case.

POJOs are Serializable

The first approach is to teach your POJOs how they are being serialized. That means that they implement some interface, like CsvSerializable which defines the method `serialize(CSVWriter csvWriter).

This has the upsides that the the objects themselves know how to serialize themselves. The downsides are that your data is now bound to CSVWriter and that you need to thing about where and how to put the header.

Adapters

The second possibility is to use "adapters", which means that you have Hero and HeroSerializer classes. The later has a method like serialize(Hero hero, CSVWriter csvWriter).

This has the upside that your data classes are fine on their own, the downside is that you need two classes per class (that's 100% more class per class) and that the logic you already have is not that much improved.

Monolithic serializer

That's basically that approach that you already have. With some minor changes (like encapsulating more logic into separate methods, for example the writing of each different item type into its own method) it's actually a very good approach. It keeps your data classes on their own, but you have an ever growing monolith.

Reflective serialization

Fields

Now comes the fun one, you could use reflection to access the fields at runtime. That means that your ITEM_LIST_HEADER actually becomes the key to serialization of your objects. That approach is more complicated, but means a little bit more flexibility. The downside is that your fields always must be the same than the headers...well, if you consider that a downside.

So what you could be doing is something like this:

private void writeLine(CSVWriter csvWriter, Object object, String[] fieldNames) {
    Map<String, Field> fields = getFields(object.getClass());
    String[] values = new String[fields.length];
    
    for (int index = 0; index < fieldNames.length; index++) {
        String currentFieldName = fieldNames[index];
        Field field = fields.get(currentFieldName);
        
        if (field != null) {
            values[index] = Objects.toString(field.getObject(object));
        } else {
            values[index] = "-";
        }
    }
    
    csvWriter.writeNext(values);
}

private Map<String, Field> getFields(Class<?> clazz) {
    // Could be cached for each class if performance is important.
    Map<String, Field> fields = new HashMap<>();
    
    for (Field field : clazz.getDeclaredFields()) {
        // Maybe add some checks, like if the field is static or not.
        field.setAccessible(true);
        fields.put(field.getName(), field);
    }
    
    return fields;
}

Untested and without exception handling, but I guess you see where this is going.

Getters/Setters

Like above, you can access the Getters/Setters reflectively. It is close to the same solution as above, with the difference that you don't need the set the fields accessible. However, you must be slightly more selective with what methods you expose in that case, and you need to convert the names.

\$\endgroup\$
4
  • \$\begingroup\$ Last option seems great. But from what I see my header would be bound to the data. And I would like to be able to save Weapon and Food to the same table. And for example when Food does not have field called dmg just write -. I was thinking more about creating CsvExportable interace. I will write it down under first post \$\endgroup\$
    – Suule
    Sep 17, 2020 at 18:51
  • \$\begingroup\$ Regarding the last option: the fields are private and therefore not readable. But as Suule mentioned in the question, there are getters and setters, therefore the pojos adhere to the java bean standard. In that case, better use a java.beans.Introspector to fetch the BeanInfo and then go over the properties from there. \$\endgroup\$
    – mtj
    Sep 18, 2020 at 11:35
  • \$\begingroup\$ @mtj You can always set them accessible. Which is, of course, depending on the environment, but most of the time not a problem. But yes, using the Getters/Setters is also an option in this case. I should most likely add that to my answer, I omitted it originally because I'd rather use the fields. \$\endgroup\$
    – Bobby
    Sep 18, 2020 at 17:29
  • \$\begingroup\$ @Suule Yes, or you define a header field as containing a title and a field name. A simple class for that would do the job and it is still easily readable. \$\endgroup\$
    – Bobby
    Sep 18, 2020 at 17:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.