pretty printing support for json formatter

[os97673]

This changes are needed for for cucumber/common#197

[aslakhellesoy]

Nice one. I think it would be cleaner if we passed a boolean instead of a map. The map only ever contains a pretty key, so a boolean should be sufficient.

WDYT?

[os97673]

Well, we could pass just boolean, but hash allows us add more params later w/o changing interface.

[aslakhellesoy]

What more parameters do we need?

[os97673]

Right now pretty is the only option we need, but at some point we may want to provide more (such as indent size).
I agree that hash as param is not usual for Java, but as far as I can see having hash param is rather standard in Ruby.

[aslakhellesoy]

I prefer not to speculate what we might need in the future and make it more complicated just in case we need something. DTSTTCPW ;-)

[mattwynne]

As a reader of the client code, I find the named argument more readable than just a true/false. It would be nice to preserve this readability somehow if we change the API to use a Boolean.

[os97673]

@mattwynne I agree: named arguments are more readable.
@aslakhellesoy why do you think that hash param complicates the API/implementation.
If you do not like that java version now takes Map we could add ctor which will take just boolean and add comment that ctor with Map is only supposed to be used from JRuby. Will this help?

[aslakhellesoy]

As a reader of the client code, I find the named argument more readable than just a true/false.

Is there something about this method that makes it less readable than any other method using normal arguments?
On the contrary I could argue that you have to read the method implementation to find out what options you can pass - looking at the signature doesn't reveal it.

why do you think that hash param complicates the API/implementation.

The caller has to read the implementation, and if he accidentally passes a map where the value is not a Boolean, he'd get a ClassCastException. It's also not very idiomatic Java.

I think the simplest thing to do in this case is to not add an extra argument at all - just always make pretty JSON. Minified JSON only makes sense when it's really big and it's sent over the wire. That's not the case here.

[os97673]

I think the simplest thing to do in this case is to not add an extra argument at all - just always make pretty JSON. Minified JSON only makes sense when it's really big and it's sent over the wire. That's not the case here.

I like this idea.

[mattwynne]

On 16 Mar 2013, at 10:04, Oleg Sukhodolsky [email protected] wrote:

I think the simplest thing to do in this case is to not add an extra argument at all - just always make pretty JSON. Minified JSON only makes sense when it's really big and it's sent over the wire. That's not the case here.

I like this idea.

+1

[os97673]

Ok, then I will implement this.