Added a check for null.  Negative money, in principal, is valid, so it depends on Mambu requirements for this api. I guess it's expected to be positive in this case but I left it as is for now, relying on Mambu to validate the business requirements part.  I fully agree, it does make sense to have  the amount being passed as BigDecimal or as Mambu Money.  This sort of things, unfortunately. are present in a whole bunch of existent code already (both loans and savings). For example, Loan's applyFee has exactly the same signature. It might be easier to change them all at the same time, with possibly the unit tests changes effort, demo changes, testing, external code, ,etc. 

On May 23, 2014, at 2:31 AM, thobach <notifications@github.com> wrote:

> In src/com/mambu/apisdk/services/SavingsService.java:
> 
> > +	 * @param amount
> > +	 *            fee amount
> > +	 * @param notes
> > +	 *            transaction notes
> > +	 * 
> > +	 * @return Savings Transaction
> > +	 * 
> > +	 * @throws MambuApiException
> > +	 */
> > +	public SavingsTransaction applyFeeToSavingsAccount(String accountId, String amount, String notes)
> > +			throws MambuApiException {
> > +
> > +		if (accountId == null || accountId.trim().isEmpty()) {
> > +			throw new IllegalArgumentException("Account ID must not  be null or empty");
> > +		}
> > +
> Please add some checks on the amount well, e.g. non-null and above 0.
> Doesn't it make sense to take make the amount param of type a BigDecimal?
> 
> —
> Reply to this email directly or view it on GitHub.
>