Conversation
It was difficult to test ToopherAPI because ToopherAPI#authenticate instantiated AuthenticationStatus objects. It's now possible to provide an AuthenticationStatusFactory to ToopherAPI. To facilitate optional arguments to ToopherAPI, there's now a ToopherAPI builder that all of the existing constructors dump their arguments into, and a ToopherAPI constructor which takes a ToopherAPI.Builder. There's a mock factory and an example test (that should be renamed or removed) that demonstrates the use of these new classes.
* feature/builders: Add factories and builders all over the places Conflicts: src/test/java/com/toopher/TestToopherAPI.java
There was a problem hiding this comment.
Why not check for null and not empty for this and the following property?
There was a problem hiding this comment.
This is code from ToopherAPI that I refactored to use builders and factories. There wasn't a null check in the original code and I didn't want to add a null check for fear of changing the behavior, since we didn't have tests around it.
There was a problem hiding this comment.
What would the json parameter contain in this case? Without reviewing the code more thoroughly this method seems like it would simply create an object from json then set the same properties again.
There was a problem hiding this comment.
json is the JSONObject, like AuthenticationStatus#_raw_data (or whatever) in toopher-python. Off the top of my head, the only thing that the call to super does is save json as an instance variable.
5 participants
Adds tests. Increases test coverage from 30% to 70% 😸 See https://coveralls.io/r/toopher/toopher-java