Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

See the previous and initial iteration. See the next iteration.

I have mainly refactored the code and made it a little bit robust (if serialization of a single object produces a string with new line character(s), an exception is thrown as the deserialization routine relies on assumption that the object string representations are separated by new line characters).

My code at this point:

StringSerializer.java:

package net.coderodde.lists.serial;

import java.util.List;

/**
 * This interface defines the API for serializing an object to a string.
 * 
 * @author Rodion "rodde" Efremov
 * @version 1.6
 * @param <E> the element type.
 */
@FunctionalInterface
public interface StringSerializer<E> {

    /**
     * Returns the textual representation of the input object.
     * 
     * @param  element the object to serialize.
     * @return the textual representation of the input object.
     */
    public String serialize(E element);

    /**
     * Serializes all the objects. Each object should serialize to a single
     * line, as the deserialization routine assumes that each line encodes 
     * entirely a single object.
     * 
     * @param <E>        the actual object type.
     * @param list       the list of objects to serialize.
     * @param serializer the object serializer.
     * @return           the string representation of the entire list.
     * @throws IllegalArgumentException if the serialization string of an 
     *                                  object contains a new line character.
     */
    public static <E> String serialize(List<E> list, 
                                       StringSerializer<E> serializer) {
        StringBuilder sb = new StringBuilder();

        for (E element : list) {
            String text = serializer.serialize(element);

            if (text.contains("\n")) {
                throw new IllegalArgumentException(
                "The serialization string of an object contains a new line " + 
                "character.");
            }

            sb.append(text).append("\n");
        }

        return sb.toString();
    }
}

StringDeserializer.java:

package net.coderodde.lists.serial;

import java.util.ArrayList;
import java.util.List;
import java.util.Scanner;

/**
 * This interface defines the API for deserializing the elements from their 
 * textual representation and provides a method for deserializing the lists.
 * 
 * @author Rodion "rodde" Efremov
 * @version 1.6
 * @param <E> the element type.
 */
@FunctionalInterface
public interface StringDeserializer<E> {

    /**
     * Deserializes an element from its textual representation.
     * 
     * @param  text the string representing the state of the object.
     * @return the actual, deserialized object.
     */
    public E deserialize(String text);

    /**
     * Deserializes the entire text <code>text</code> to the list of objects 
     * being encoded.
     * 
     * @param <E>          the actual element type.
     * @param text         the text to deserialize.
     * @param deserializer the deserialization object.
     * @return             the list of elements.
     */
    public static <E> List<E> deserialize(String text, 
                                          StringDeserializer<E> deserializer) {
        List<E> ret = new ArrayList<>();
        Scanner scanner = new Scanner(text);

        while (scanner.hasNextLine()) {
            ret.add(deserializer.deserialize(scanner.nextLine()));
        }

        return ret;
    }
}

Demo.java:

package net.coderodde.lists.serial;

import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.Random;

public class Demo {

    public static void main(String[] args) {
        // Create.
        List<Integer> input = getRandomInts(100, new Random());
        // Serialize.
        String text = StringSerializer.serialize(input, (e) -> e.toString());
        // Deserialize.
        List<Integer> output = 
                StringDeserializer.deserialize(text, 
                                               (e) -> Integer.parseInt(e));

        System.out.println("Input list size:  " + input.size());
        System.out.println("Output list size: " + output.size());

        for (int i = 0; i < input.size(); ++i) {
            if (!Objects.equals(input.get(i), output.get(i))) {
                throw new IllegalStateException("Lists do not agree! :-[");
            }
        }

        System.out.println("Lists agree! :-]");
    }

    private static List<Integer> getRandomInts(int size, Random random) {
        List<Integer> ret = new ArrayList<>();

        for (int i = 0; i < size; ++i) {
            ret.add(random.nextInt());
        }

        return ret;
    }
}

So, what do you think?

share|improve this question
2  
Hey, I remember you! Lemme take another stab at this baby. –  QPaysTaxes Jun 22 at 1:44
    
Looks much simpler now. ++ –  RubberDuck Jun 22 at 10:35

2 Answers 2

up vote 2 down vote accepted

You can use the following to generate random Integers:

private static List<Integer> getRandomInts(int size, Random random) {
    return random.ints(size).boxed().collect(Collectors.toList());
}

Scanner scanner = new Scanner(text) can be wrapped as a try-with-resources statement:

try (Scanner scanner = new Scanner(text)) {
    while (scanner.hasNextLine()) {
        ret.add(deserializer.deserialize(scanner.nextLine()));
    }
    return ret;
}

However, as @QPayTaxes has pointed out, you probably don't have to worry about errors when parsing a non-I/O-based resource, which is also mentioned in the API note of AutoCloseable. Hence, it is safe to ignore the warning your IDE might warn you about for the Scanner instance not being closed, or simply do a scanner.close() at the end to make the warning go away.

Your (e) -> e.toString() and (e) -> Integer.parseInt(e) lambdas can simply be written as Object::toString and Integer::parseInt.

You do not need to iterate through your Lists to check each element, as List.equals() takes care of that.

Finally, I think it'll be better if you can come up with better unit tests to ascertain that the different parts of your code is working correctly. For example, you can use a custom serializer implementation that adds newlines to test that your StringSerializer.serialize() correctly throws an IllegalArgumentException.

share|improve this answer
    
Why a try-with-resources when you can just use scanner.close()? It's not like you're gonna throw an error while trying to open or read from a scanner on a String in memory. –  QPaysTaxes Jun 22 at 1:56
    
@QPaysTaxes fair point, I'll update to include that. –  h.j.k. Jun 22 at 2:06
    
...Dang, that's already in my (as-yet-unposted) answer –  QPaysTaxes Jun 22 at 2:07
1  
One of the main reasons for try-with-resources is so that it liberates you from manually closing (and often forgetting to close) auto closeables. It's a good thing –  janos Jun 22 at 5:27

This serialization framework has some major flaws:

  • The record separator is not obvious from the class names and public methods. Users are forced to read the implementation to find out this important detail. This is a failure of good encapsulation principles

  • Looking at the StringSerializer interface, since it's not obvious that newlines are forbidden, implementers might violate the rule without knowing, and learn about the problem at runtime, which is too late, as opposed to compile time

  • The serializer / deserializer logic must agree on the record separator, but this is not obvious from the class names and public methods, and the framework cannot enforce it. One solution is to

  • Interface methods are designed to be implemented or overridden. Therefore I think a static method on an interface violates good practices

  • I suppose it's by design, but this cannot serialize nested objects such as list of lists

How to do it better?

  • Move the static methods out of the interfaces to a factory. This will make the interfaces cleaner, leaving only methods to implement

  • If both static methods are implemented on the same factory, it becomes obvious that they agree on the record separator

  • The name of the factory could be worded in a way to imply the newline as record separator, to give users a clue without having to verify the implementation

  • Alternatively, you could add an argument to the serializer method with the set of forbidden characters to make implementers conscious about this restriction. A softer alternative is to add an accessor to the factory method so that users can query the forbidden characters

    • If these options don't sound great, you're right. The serialization framework of the JDK doesn't do it this way either

Ultimately, I suggest to look at the source code of the JDK for better ways to implement serialization, for example on grepcode.com

share|improve this answer
    
"One solution is to" I think you accidentally a solution... Unless I'm misreading. Also, it can serialize lists of lists, but you have to supply the serializer yourself. Aside from that, you stole most of my answer, meany! Maybe I should have posted it instead of saving it for later. Oh well. –  QPaysTaxes Jun 22 at 16:43
    
See the 2nd bullet in your answer. I see no means by which I can make sure that the user's serializer does not include the new line char at compile time. –  coderodde Jun 22 at 17:30
    
And what package should I take a look on at grepcode.com? –  coderodde Jun 22 at 17:33
1  
For example ObjectOutputStream for serialization and ObjectInputStream for deserialization. (You might hate me forever for this suggestion.) –  janos Jun 22 at 18:27

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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