cdevents / sdk-java

Java SDK for CDEvents
Apache License 2.0
5 stars 6 forks source link

Should use lombok for generating `toString` and `hashCode` #68

Open xibz opened 8 months ago

xibz commented 8 months ago

Right now model classes do not override these methods making, equals, a little more complicated. Lombok can make this easy.

rjalander commented 7 months ago

I am using a maven plugin jsonschema2pojo here to generate the model classes, not sure we can use Lombok with jsonschema2pojo

Today, here is the configuration to generate toStringand hashCode using this plugin - https://github.com/cdevents/sdk-java/blob/main/generator/pom.xml#L92-L93

xibz commented 7 months ago

If you couldn't do this, Id be very questionable about the library

https://github.com/joelittlejohn/jsonschema2pojo/issues/524 but it looks like you can

aalmiray commented 6 months ago

If you couldn't do this, Id be very questionable about the library

That is not a nice thing to say. Please be respectful.

As @rjalander noted, generated code via schema2pojo does add equals and hashCode but toString is currently disabled. See

https://github.com/cdevents/sdk-java/blob/9f4e193dc1ee8a920129f84bd1b79e67cd417527/generator/pom.xml#L72-L79

@rjalander perhaps <includeToString> may be set to true?

Also, @xibz, even though I'm a fan of Lombok it doesn't make sense to add it to this project, nor any other source code generator driven by an annotation processor because this project uses its own source code generator.

rjalander commented 6 months ago

I did not include toString because in these unit tests, I am comparing the expected and actual objects instead of strings for the generated events

aalmiray commented 6 months ago

toString is also useful for logging. Usually it's not a good idea to compare equality using literals, that is why Java has the equals method 😏

rjalander commented 6 months ago

Yes the equals methods is implemented to compare the content of the objects in all the generated models, so it should produce the same result when comparing with objects too.

assertTrue(expectedEvent.getContext().equals(createdEvent.getContext()));
OR
assertEquals(expectedEvent.getContext(), createdEvent.getContext());

But I think we can still implement toString() method too, as it invokes only equals method in either case unless explicitly invoked.