search

hawkular / hawkular-client-java

Java client for Hawkular
Apache License 2.0
11 stars 8 forks source link

Fixed null pointer exception and added wrapper files for inventory #3

Closed jkandasa closed 9 years ago

jkandasa commented 9 years ago

@vnugent Could you please review the changes.

vnugent commented 9 years ago

Thanks for the PR! We should implement the plumbing for REST in this project and provide a layer of abstraction to the client's consumer. Another word the inventory test shouldn't have to deal directly with REST API. Please see TenantTest.java

vnugent commented 9 years ago

It may seem like we're having extra code for nothing but the extra layer, Inventory client, Metrics client class can help deal with Rest-related peculiarity. For example, https://github.com/Hawkular-QE/hawkular-java-client/blob/master/src/main/java/org/hawkular/client/metrics/MetricsClientImpl.java#L42

jkandasa commented 9 years ago

I think the extra layer might give extra work, if there is a change on hawkular api side, however we can protect client side existing code by changing on abstraction class.

jkandasa commented 9 years ago

How will you handle other than boolen, collection and String objects with abstraction class? For example, MetricDefinition, IdWrapper. For these class if you generate object and return, while accessing inside methods in that object throws null pointer exception. As this abstraction class could not avoid null pointer exception, I guess this extra abstract class is not required.