Thomas, I made the change as suggested. Though I don't think it's an OO issue. With your proposal the connivence methods are not  'connivence' only any more :)  They are all really required and would need to be also written for all current and future services that support attachments api (branches, users, centres, products, etc. ).  The implementation that I proposed was with the intent to be able use this DocumentService method to get documents for any type of Mambu entity in a future without writing any more code.   Just to clarify the intent  :).

 I also removed public getMambuAPIService()  from all services as requested. I believe that (with no class relationship between all these service classes)  some sort of mechanism would be needed to support good refectoting, which, I agree, is really, really needed. Possibly, creating a super-class or a common helper utility, or some methods to identify each service, etc.. I can give it a try in between the api changes submissions :)

On May 23, 2014, at 2:07 AM, thobach <notifications@github.com> wrote:

> In src/com/mambu/apisdk/services/DocumentsService.java:
> 
> > +	 * See {@link https://mambucom.jira.com/browse/MBU-5084} for more details
> > +	 * 
> > +	 * @param mambuAPIService
> > +	 *            Mambu Api service to use for executing API request
> > +	 * 
> > +	 * @param serviceEndPoint
> > +	 *            the API endpoint string for this service. E.g. APIData.CLIENT, APIData.LOANS, etc.
> > +	 * 
> > +	 * @param entityId
> > +	 *            the encoded key or id of the Mambu entity for which the attached documents are to be retrieved
> > +	 * 
> > +	 * @return documents attached to the entity
> > +	 * 
> > +	 * @throws MambuApiException
> > +	 */
> > +	public static List<Document> getDocuments(MambuAPIService mambuAPIService, String serviceEndPoint, String entityId)
> The helper methods are a good idea, but I'd make this method only protected, non-static, remove the mambuAPIService param and then remove the getMambuAPIService() method from each service. In the convenience methods you can then call new DocumentsService (mambuAPIService).getDocuments(..., ...). This is more OO in my opinion and exposes less internals, which should not be required by external code anyways.
> 
> —
> Reply to this email directly or view it on GitHub.
>