patternjavaMinor

Loading card data from XML file

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

filecardxmlloadingfromdata

Problem

For our Cardshifter Project we wanted to be able to load card definitions from file.

A card is defined as:

Having an unique id.

Having a set of resources, which are integers identified by a string.

Having a set of attributes, which are strings identified by a string.

In the implementation every card gets mapped to an Entity in our Entity-Component Framework, however that is to be considered an implementation detail and is not the intended focus of this review.

This code is also accompanied with tests, but as there are lots of tests, they require Entity-Component Framework knowledge and are all accompanied with their own XML file, they are also not included.

You can review all code at our Github project page on the develop branch and the tests can be found here.

Then now follows the implementation code:

CardLoadingException class

public class CardLoadingException extends Exception {
    private static final long serialVersionUID = 64626564562564598L;

    public CardLoadingException() {

    }

    public CardLoadingException(final String message) {
        super(message);
    }

    public CardLoadingException(final Throwable cause) {
        super(cause);
    }

    public CardLoadingException(final String message, final Throwable cause) {
        super(message, cause);
    }
}

UncheckedCardLoadingException class

class UncheckedCardLoadingException extends RuntimeException {
    private static final long serialVersionUID = 6644614691255149498L;

    UncheckedCardLoadingException() {

    }

    UncheckedCardLoadingException(final String message) {
        super(message);
    }

    UncheckedCardLoadingException(final Throwable cause) {
        super(cause);
    }

    UncheckedCardLoadingException(final String message, final Throwable cause) {
        super(message, cause);
    }
}

CardLoader interface

```
/**
* An interface to be implemented by cardloaders.
*
* @author Frank van Heeswijk
* @param The input file type of

Solution

Readability

For me this is clearly to many lines of code inside this method. So let us see, what you are doing.

you are creating a CardInfo object by using the Path path.

you are creating a List's and validate them

you are createing Map




As it seems 1. and 2. won't change in the near future but add a lot of codelines to this method. So let us extract them to separate methods.  

private CardList readFromFile(File file)
{
    SAXBuilder saxBuilder = new SAXBuilder();
    Document document = saxBuilder.build(file);

    XMLOutputter xmlOutputter = new XMLOutputter(Format.getCompactFormat().setExpandEmptyElements(true));
    String unformattedXmlString = xmlOutputter.outputString(document);

    JacksonXmlModule xmlModule = new JacksonXmlModule();
    xmlModule.setDefaultUseWrapper(false);
    ObjectMapper xmlMapper = new XmlMapper(xmlModule);

    return xmlMapper.readValue(unformattedXmlString, CardInfo.class);
}

private List getValidatedTags(List resourcesList, List attributesList)
{
    List tags = Stream.concat(resourcesList.stream(), attributesList.stream())
        .map(ecsElement -> sanitizeTag(ecsElement.toString()))
        .collect(Collectors.toList());

    validateTags(tags);

    return tags;
}  

private void validateTags(List tags)
{

    if (requiredTags().stream().anyMatch(tags::contains)) {
        throw new UncheckedCardLoadingException("Tags " + requiredTags() + " are required by default you cannot submit them in the resources or attributes.");
    }

    List duplicateTags = tags.stream()
        .collect(Collectors.groupingBy(i -> i))
        .entrySet()
        .stream()
        .filter(entry -> entry.getValue().size() > 1)
        .map(Entry::getKey)
        .collect(Collectors.toList());

    if (!duplicateTags.isEmpty()) {
        throw new UncheckedCardLoadingException("Tags " + duplicateTags + " have been input multiple times, this is not allowed.");
    }

}

private List getValidatedCards(CardInfo cardInfo)
{
    List cards = cardInfo.getCards().getCards();

    List duplicateIds = cards.stream()
        .collect(Collectors.groupingBy(Card::getId))
        .entrySet()
        .stream()
        .filter(entry -> entry.getValue().size() > 1)
        .map(Entry::getKey)
        .collect(Collectors.toList());

    if (!duplicateIds.isEmpty()) {
        throw new UncheckedCardLoadingException("Card ids " + duplicateIds + " are duplicaties, this is not allowed.");
    }

    return cards;
}


Now, after renaming

cardList to cards and rearanging the creating of the Map's the former loadCards()` method looks like

@Override
public Collection loadCards(final Path path, final Supplier entitySupplier, final ECSResource[] resources, final ECSAttribute[] attributes) throws CardLoadingException {
    Objects.requireNonNull(path, "path");
    Objects.requireNonNull(entitySupplier, "entitySupplier");

    List resourcesList = (resources == null) ? Arrays.asList() : Arrays.asList(resources);
    List attributesList = (attributes == null) ? Arrays.asList() : Arrays.asList(attributes);

    try {

        CardInfo cardInfo = readFromFile(path.toFile());

        List tags = getValidatedTags(resourcesList, attributesList);

        List cards = getValidatedCards(cardInfo);

        Map ecsResourcesMap = resourcesList.stream()
            .collect(Collectors.toMap(ecsResource -> sanitizeTag(ecsResource.toString()), i -> i));

        Map ecsAttributesMap = attributesList.stream()
            .collect(Collectors.toMap(ecsAttribute -> sanitizeTag(ecsAttribute.toString()), i -> i));

        return cards.stream()
            .map(card -> {
                Entity entity = entitySupplier.get();

                entity.addComponent(new IdComponent(card.getId()));

                ECSResourceMap resourceMap = ECSResourceMap.createFor(entity);
                ECSAttributeMap attributeMap = ECSAttributeMap.createFor(entity);

                card.getElements().forEach((sanitizedTag, value) -> {
                    if (ecsResourcesMap.containsKey(sanitizedTag)) {
                        resourceMap.set(ecsResourcesMap.get(sanitizedTag), Integer.parseInt(value.toString()));
                    }
                    else if (ecsAttributesMap.containsKey(sanitizedTag)) {
                        attributeMap.set(ecsAttributesMap.get(sanitizedTag), value.toString());
                    }
                    else {
                        throw new UncheckedCardLoadingException("Element " + sanitizedTag + " has not been found in the supplied resource and attribute mappings where card id = " + card.getId());
                    }
                });

                return entity;
            })
            .collect(Collectors.toList());
    } catch (UncheckedCardLoadingException ex) {
        throw new CardLoadingException(ex.getMessage(), ex.getCause());
    } catch (Exception ex) {
        throw new CardLoadingException(ex);
    }
}

What you are needed to do is add the

Code Snippets

private CardList readFromFile(File file)
{
    SAXBuilder saxBuilder = new SAXBuilder();
    Document document = saxBuilder.build(file);

    XMLOutputter xmlOutputter = new XMLOutputter(Format.getCompactFormat().setExpandEmptyElements(true));
    String unformattedXmlString = xmlOutputter.outputString(document);

    JacksonXmlModule xmlModule = new JacksonXmlModule();
    xmlModule.setDefaultUseWrapper(false);
    ObjectMapper xmlMapper = new XmlMapper(xmlModule);

    return xmlMapper.readValue(unformattedXmlString, CardInfo.class);
}

private List<String> getValidatedTags(List<ECSResource> resourcesList, List<ECSAttribute> attributesList)
{
    List<String> tags = Stream.concat(resourcesList.stream(), attributesList.stream())
        .map(ecsElement -> sanitizeTag(ecsElement.toString()))
        .collect(Collectors.toList());

    validateTags(tags);

    return tags;
}  

private void validateTags(List<String> tags)
{

    if (requiredTags().stream().anyMatch(tags::contains)) {
        throw new UncheckedCardLoadingException("Tags " + requiredTags() + " are required by default you cannot submit them in the resources or attributes.");
    }

    List<String> duplicateTags = tags.stream()
        .collect(Collectors.groupingBy(i -> i))
        .entrySet()
        .stream()
        .filter(entry -> entry.getValue().size() > 1)
        .map(Entry::getKey)
        .collect(Collectors.toList());

    if (!duplicateTags.isEmpty()) {
        throw new UncheckedCardLoadingException("Tags " + duplicateTags + " have been input multiple times, this is not allowed.");
    }

}

private List<Card> getValidatedCards(CardInfo cardInfo)
{
    List<Card> cards = cardInfo.getCards().getCards();

    List<String> duplicateIds = cards.stream()
        .collect(Collectors.groupingBy(Card::getId))
        .entrySet()
        .stream()
        .filter(entry -> entry.getValue().size() > 1)
        .map(Entry::getKey)
        .collect(Collectors.toList());

    if (!duplicateIds.isEmpty()) {
        throw new UncheckedCardLoadingException("Card ids " + duplicateIds + " are duplicaties, this is not allowed.");
    }

    return cards;
}

@Override
public Collection<Entity> loadCards(final Path path, final Supplier<Entity> entitySupplier, final ECSResource[] resources, final ECSAttribute[] attributes) throws CardLoadingException {
    Objects.requireNonNull(path, "path");
    Objects.requireNonNull(entitySupplier, "entitySupplier");

    List<ECSResource> resourcesList = (resources == null) ? Arrays.asList() : Arrays.asList(resources);
    List<ECSAttribute> attributesList = (attributes == null) ? Arrays.asList() : Arrays.asList(attributes);

    try {

        CardInfo cardInfo = readFromFile(path.toFile());

        List<String> tags = getValidatedTags(resourcesList, attributesList);

        List<Card> cards = getValidatedCards(cardInfo);

        Map<String, ECSResource> ecsResourcesMap = resourcesList.stream()
            .collect(Collectors.toMap(ecsResource -> sanitizeTag(ecsResource.toString()), i -> i));

        Map<String, ECSAttribute> ecsAttributesMap = attributesList.stream()
            .collect(Collectors.toMap(ecsAttribute -> sanitizeTag(ecsAttribute.toString()), i -> i));

        return cards.stream()
            .map(card -> {
                Entity entity = entitySupplier.get();

                entity.addComponent(new IdComponent(card.getId()));

                ECSResourceMap resourceMap = ECSResourceMap.createFor(entity);
                ECSAttributeMap attributeMap = ECSAttributeMap.createFor(entity);

                card.getElements().forEach((sanitizedTag, value) -> {
                    if (ecsResourcesMap.containsKey(sanitizedTag)) {
                        resourceMap.set(ecsResourcesMap.get(sanitizedTag), Integer.parseInt(value.toString()));
                    }
                    else if (ecsAttributesMap.containsKey(sanitizedTag)) {
                        attributeMap.set(ecsAttributesMap.get(sanitizedTag), value.toString());
                    }
                    else {
                        throw new UncheckedCardLoadingException("Element " + sanitizedTag + " has not been found in the supplied resource and attribute mappings where card id = " + card.getId());
                    }
                });

                return entity;
            })
            .collect(Collectors.toList());
    } catch (UncheckedCardLoadingException ex) {
        throw new CardLoadingException(ex.getMessage(), ex.getCause());
    } catch (Exception ex) {
        throw new CardLoadingException(ex);
    }
}

@param <I>   The input file type of the card loader on each individual load action

@param <I>   The input source of the card loader on each individual load action

Collection<Entity> loadCards(I source, ...)

Context

StackExchange Code Review Q#69092, answer score: 4

Revisions (0)

No revisions yet.