Suggested API

[mishako]

Main ideas:

• Single object for reading and writing
• Use builder for configuration
• Wrap parsed raw values

The entrypoint is class Rome with only two methods:

static Engine.Builder custom() { return Engine.builder(); }
static Engine minimal() { return custom().build(); }

Engine.Builder is used for configuration. For example:

Engine rome = Rome.custom()
  .set(WriteConfig.FORMAT, Format.JSON_FEED)
  .build()

Now when you have a configured Engine you can read a feed:

Feed feed = rome.read(inputStream);
StringValue title = feed.title();
DateTimeValue published = feed.published();

StringValue and DateTimeValue are wrappers around String and DateTime. They have additional information attached, e.g. raw values, parsing errors, location in the XML.

To write a feed you build it and pass it to your Engine:

Feed feed = Feed.builder()
  .title("test")
  .published(DateTime.now())
  .build();
rome.write(feed, outputStream);

See more examples in my gist.

[mishako]

In rometools/rome#353 @buko said:

Builders are IMO an antipattern. I would recommend (1) immutable interfaces and (2) factories that can be configured.

One thing to keep in mind is that today many ppl use DI. This means it's much easier to inject a factory and use that than to prototype builders. Extensions for examples can just be factories.

I'm not sure I understand the difference between a builder and a factory from DI perspective. I mean, in case of a builder wouldn't you build the object beforehand and inject an already built object?

[imk]

About the comment from @buko

I don't think that builders are an antipattern and configurable factories should be preferred, builders are configurable factories with the factory method build().

Generally, builders are used to create a (more or less) complex instance of a concrete class, where as a factory may create different implementations. Like Feed.builder().build() should allways create instances of the class Feed (and not subclasses of Feed or classes implementing an interface Feed) whereas a FeedFactory may create implementations of RssFeed or AtomFeed.

tl;dr If you want to create immutable instances of a concrete class, call it Builder. If you wan't to create instances of different implementations or inherited classes depending on the actual configuration, call it Factory.

[imk]

Some questions or paraphrasing about the interface to be sure I'll get it right:

• instances of the class Rome (or Engine):
  • are responsible for reading and writing feeds
  • are created by static methods providing either a default instance or a builder to customize (these methods are the single entrypoint for users to work with Rome)
  • are immutable (and so should be threadsafe, can be reused etc)
  • can read any feed format
  • are configured to write just one format (when creating RSS and Atom, I have to configure two instances)
  • YahooWeather.Feed feed = rome.read(data).as(YahooWeather.MODEL); I don't get why that's different from Itunes example, why not YahooWeather.Feed weatherFeed = feed.as(YahooWeather.MODEL);.
  • how about creation of builders based on an existing instance? Something like Rome.builder(Rome source) to create a new instance with my personal default config but change some values.
• instances of the class Rome.Builder:
  • are created by static methods of Rome
  • are mutable
  • are creating custom configured immutable Rome instances
  • config of output format via .set(WriteConfig.FORMAT, Format.JSON_FEED): Format.JSON_FEED is an enum? That's clean but would make it harder to make feed writers pluggable. (I think about having parsers and writers pluggable like module parsers and writers in Rome 1, don't know yet how to implement that best)
  • Plugin registration (register(YahooWeather.class)): we could load plugins via ServiceLoader instead of manually registering
  • single responsibility: Configuring Rome
• instances of the class Feed:
  • mutable or immutable? Having everything immutable would be nice, in case of feed it may get overloaded and complicated having builders for every element in the feed (like when you want/have to change a value in a module of a feed entry, you would have to create a builder from the existing feed, all entries and all modules just to be able to change a value).

[mishako]

@imk You got everything right, that's how I see it.

The difference between Itunes and yahoo comes from me pretending that yahoo is an external plugin supplied by the user. The rome.read(data).as(...) and feed.as(...) are basically the same things, because rome.read(data) returns a feed.

Rome.builder(Rome src) make a lot of sense. Can also add Rome.builder(Rome.Builder src).

Not sure if Format.JSON_FEED is enum. I guess it could be a constant pointing to a plugin object that handles this particular format. Not sure if we should try to be efficient and not create these kind of objects beforehand.

I don't know much about ServiceLoader, but it sounds like a good idea. We could make it optional, similarly to how jackson does it: ObjectMapper.java#L978.

Not sure how to solve inconvenience with updating an immutable object. Maybe something like this?

Feed feed = rome.read(data);
Feed.Builder feedBuilder = feed.toBuilder(); // Converts whole object tree to builders 
List<Item.Builder> items = feedBuilder.items(); // Items are actually builders now
items.forEach(item -> item.title("test")); // Update titles
Feed feed2 = feedBuilder.build(); // Builds whole object tree

Or we could provide a map method:

Feed feed2 = feed.map(builder -> builder.items().forEach(item -> item.title("test")));

[imk]

@mishako that's looking good, should work that way! 👍

[mishako]

Doesn't seem to be controversial. Let's try it out in the prototype. I'm closing the ticket.