MIFOSX-109: some formatting changes and removal of unused items.

mifosng-provider/src/main/java/org/mifosng/platform/api/GuarantorsApiResource.java

@@ -47,11 +47,9 @@ public class GuarantorsApiResource {
     @Autowired
     private PortfolioApiJsonSerializerService apiJsonSerializerService;
 
-	private static final Set<String> typicalResponseParameters = new HashSet<String>(
+    private static final Set<String> typicalResponseParameters = new HashSet<String>(Arrays.asList("externalGuarantor", "existingClientId",
-			Arrays.asList("externalGuarantor", "existingClientId", "firstname",
+            "firstname", "lastname", "addressLine1", "addressLine2", "city", "state", "zip", "country", "mobileNumber", "housePhoneNumber",
-					"lastname", "addressLine1", "addressLine2", "city",
+            "comment", "dob"));
-					"state", "zip", "country", "mobileNumber",
-					"housePhoneNumber", "comment", "dob"));
 
     private final String SystemEntityType = "m_guarantor_external";
 
@@ -61,33 +59,25 @@ public class GuarantorsApiResource {
     @GET
     @Consumes({ MediaType.APPLICATION_JSON })
     @Produces({ MediaType.APPLICATION_JSON })
-	public String retrieveGuarantorDetails(@Context final UriInfo uriInfo,
+    public String retrieveGuarantorDetails(@Context final UriInfo uriInfo, @PathParam("loanId") final Long loanId) {
-			@PathParam("loanId") final Long loanId) {
         context.authenticatedUser().validateHasReadPermission(SystemEntityType);
-		Set<String> responseParameters = ApiParameterHelper
+        Set<String> responseParameters = ApiParameterHelper.extractFieldsForResponseIfProvided(uriInfo.getQueryParameters());
-				.extractFieldsForResponseIfProvided(uriInfo
-						.getQueryParameters());
         if (responseParameters.isEmpty()) {
             responseParameters.addAll(typicalResponseParameters);
         }
-		boolean prettyPrint = ApiParameterHelper.prettyPrint(uriInfo
+        boolean prettyPrint = ApiParameterHelper.prettyPrint(uriInfo.getQueryParameters());
-				.getQueryParameters());
 
-		GuarantorData guarantorData = guarantorReadPlatformService
+        GuarantorData guarantorData = guarantorReadPlatformService.retrieveGuarantor(loanId);
-				.retrieveGuarantor(loanId);
 
-		return this.apiJsonSerializerService.serializeGuarantorDataToJson(
+        return this.apiJsonSerializerService.serializeGuarantorDataToJson(prettyPrint, responseParameters, guarantorData);
-				prettyPrint, responseParameters, guarantorData);
     }
 
     @POST
     @Consumes({ MediaType.APPLICATION_JSON })
     @Produces({ MediaType.APPLICATION_JSON })
-	public Response createGuarantor(@PathParam("loanId") final Long loanId,
+    public Response createGuarantor(@PathParam("loanId") final Long loanId, final String jsonRequestBody) {
-			final String jsonRequestBody) {
 
-		final GuarantorCommand command = this.apiDataConversionService
+        final GuarantorCommand command = this.apiDataConversionService.convertJsonToGuarantorCommand(null, loanId, jsonRequestBody);
-				.convertJsonToGuarantorCommand(null, loanId, jsonRequestBody);
 
         this.guarantorWritePlatformService.createGuarantor(loanId, command);
 
@@ -98,10 +88,8 @@ public class GuarantorsApiResource {
     @PUT
     @Consumes({ MediaType.APPLICATION_JSON })
     @Produces({ MediaType.APPLICATION_JSON })
-	public Response updateGuarantor(@PathParam("loanId") final Long loanId,
+    public Response updateGuarantor(@PathParam("loanId") final Long loanId, final String jsonRequestBody) {
-			final String jsonRequestBody) {
+        final GuarantorCommand command = this.apiDataConversionService.convertJsonToGuarantorCommand(null, loanId, jsonRequestBody);
-		final GuarantorCommand command = this.apiDataConversionService
-				.convertJsonToGuarantorCommand(null, loanId, jsonRequestBody);
 
         this.guarantorWritePlatformService.updateGuarantor(loanId, command);
 

mifosng-provider/src/main/java/org/mifosng/platform/api/LoansApiResource.java

@@ -110,11 +110,8 @@ public class LoansApiResource {
     @Path("template")
     @Consumes({ MediaType.APPLICATION_JSON })
     @Produces({ MediaType.APPLICATION_JSON })
-	public String retrieveDetailsForNewLoanApplicationStepOne(
+    public String retrieveDetailsForNewLoanApplicationStepOne(@QueryParam("clientId") final Long clientId,
-			@QueryParam("clientId") final Long clientId,
+            @QueryParam("groupId") final Long groupId, @QueryParam("productId") final Long productId, @Context final UriInfo uriInfo) {
-            @QueryParam("groupId") final Long groupId,
-			@QueryParam("productId") final Long productId,
-			@Context final UriInfo uriInfo) {
 
         context.authenticatedUser().validateHasReadPermission("LOAN");
 
@@ -125,14 +122,17 @@ public class LoansApiResource {
         Collection<LoanProductData> productOptions = this.loanProductReadPlatformService.retrieveAllLoanProductsForLookup();
         Collection<EnumOptionData> loanTermFrequencyTypeOptions = dropdownReadPlatformService.retrieveLoanTermFrequencyTypeOptions();
         Collection<EnumOptionData> repaymentFrequencyTypeOptions = dropdownReadPlatformService.retrieveRepaymentFrequencyTypeOptions();
-		Collection<EnumOptionData> interestRateFrequencyTypeOptions = dropdownReadPlatformService.retrieveInterestRateFrequencyTypeOptions();
+        Collection<EnumOptionData> interestRateFrequencyTypeOptions = dropdownReadPlatformService
+                .retrieveInterestRateFrequencyTypeOptions();
 
         Collection<EnumOptionData> amortizationTypeOptions = dropdownReadPlatformService.retrieveLoanAmortizationTypeOptions();
         Collection<EnumOptionData> interestTypeOptions = dropdownReadPlatformService.retrieveLoanInterestTypeOptions();
-		Collection<EnumOptionData> interestCalculationPeriodTypeOptions = dropdownReadPlatformService.retrieveLoanInterestRateCalculatedInPeriodOptions();
+        Collection<EnumOptionData> interestCalculationPeriodTypeOptions = dropdownReadPlatformService
+                .retrieveLoanInterestRateCalculatedInPeriodOptions();
 
         Collection<FundData> fundOptions = this.fundReadPlatformService.retrieveAllFunds();
-		Collection<TransactionProcessingStrategyData> repaymentStrategyOptions = this.dropdownReadPlatformService.retreiveTransactionProcessingStrategies();
+        Collection<TransactionProcessingStrategyData> repaymentStrategyOptions = this.dropdownReadPlatformService
+                .retreiveTransactionProcessingStrategies();
 
         final boolean feeChargesOnly = false;
         Collection<ChargeData> chargeOptions = this.chargeReadPlatformService.retrieveLoanApplicableCharges(feeChargesOnly);
@@ -141,7 +141,7 @@ public class LoansApiResource {
         LoanBasicDetailsData loanBasicDetails;
         Long officeId;
 
-		if ( clientId != null ){
+        if (clientId != null) {
             loanBasicDetails = this.loanReadPlatformService.retrieveClientAndProductDetails(clientId, productId);
             officeId = loanBasicDetails.getClientOfficeId();
         } else {
@@ -155,9 +155,9 @@ public class LoansApiResource {
         Collection<StaffData> allowedLoanOfficers = this.staffReadPlatformService.retrieveAllLoanOfficersByOffice(officeId);
 
         final LoanAccountData newLoanAccount = new LoanAccountData(loanBasicDetails, convenienceDataRequired, null, null, null, charges,
-				productOptions, loanTermFrequencyTypeOptions, repaymentFrequencyTypeOptions, 
+                productOptions, loanTermFrequencyTypeOptions, repaymentFrequencyTypeOptions, repaymentStrategyOptions,
-				repaymentStrategyOptions, interestRateFrequencyTypeOptions, 
+                interestRateFrequencyTypeOptions, amortizationTypeOptions, interestTypeOptions, interestCalculationPeriodTypeOptions,
-				amortizationTypeOptions, interestTypeOptions, interestCalculationPeriodTypeOptions, fundOptions, chargeOptions, chargeTemplate, allowedLoanOfficers,null);
+                fundOptions, chargeOptions, chargeTemplate, allowedLoanOfficers, null);
 
         return this.apiJsonSerializerService.serializeLoanAccountDataToJson(prettyPrint, responseParameters, newLoanAccount);
     }
@@ -166,9 +166,7 @@ public class LoansApiResource {
     @Path("{loanId}")
     @Consumes({ MediaType.APPLICATION_JSON })
     @Produces({ MediaType.APPLICATION_JSON })
-	public String retrieveLoanAccountDetails(
+    public String retrieveLoanAccountDetails(@PathParam("loanId") final Long loanId, @Context final UriInfo uriInfo) {
-			@PathParam("loanId") final Long loanId,
-			@Context final UriInfo uriInfo) {
 
         context.authenticatedUser().validateHasReadPermission("LOAN");
 
@@ -189,12 +187,13 @@ public class LoansApiResource {
         if (!associationParameters.isEmpty()) {
 
             if (associationParameters.contains("all")) {
-				associationParameters.addAll(Arrays.asList("repaymentSchedule", "transactions", "permissions", "convenienceData", "charges", "guarantor"));
+                associationParameters.addAll(Arrays.asList("repaymentSchedule", "transactions", "permissions", "convenienceData",
+                        "charges", "guarantor"));
             }
 
             boolean existsGuarantor = false;
             if (associationParameters.contains("guarantor")) {
-				if(guarantorReadPlatformService.existsGuarantor(loanId)){
+                if (guarantorReadPlatformService.existsGuarantor(loanId)) {
                     guarantorData = this.guarantorReadPlatformService.retrieveGuarantor(loanId);
                     existsGuarantor = true;
                 }
@@ -213,16 +212,21 @@ public class LoansApiResource {
                 repaymentSchedule = this.loanReadPlatformService.retrieveRepaymentSchedule(loanId, loanBasicDetails.getCurrency(),
                         singleDisbursement, loanBasicDetails.getTotalDisbursementCharges(), loanBasicDetails.getInArrearsTolerance());
 
-				// FIXME - KW - Waive feature was changed to waive interest at anytime so this permission checking is probably not needed - look into.
+                // FIXME - KW - Waive feature was changed to waive interest at
-				final MonetaryCurrency currency = new MonetaryCurrency(loanBasicDetails.getCurrency().getCode(), loanBasicDetails.getCurrency().getDecimalPlaces());
+                // anytime so this permission checking is probably not needed -
+                // look into.
+                final MonetaryCurrency currency = new MonetaryCurrency(loanBasicDetails.getCurrency().getCode(), loanBasicDetails
+                        .getCurrency().getDecimalPlaces());
                 final Money tolerance = Money.of(currency, loanBasicDetails.getInArrearsTolerance());
                 final Money totalOutstandingMoney = Money.of(currency, repaymentSchedule.totalOutstanding());
 
-				boolean isWaiveAllowed = totalOutstandingMoney.isGreaterThanZero() && (tolerance.isGreaterThan(totalOutstandingMoney) || tolerance.isEqualTo(totalOutstandingMoney));
+                boolean isWaiveAllowed = totalOutstandingMoney.isGreaterThanZero()
+                        && (tolerance.isGreaterThan(totalOutstandingMoney) || tolerance.isEqualTo(totalOutstandingMoney));
 
                 if (associationParameters.contains("permissions")) {
                     loanRepaymentsCount = retrieveNonDisbursementTransactions(loanRepayments);
-					permissions = this.loanReadPlatformService.retrieveLoanPermissions(loanBasicDetails, isWaiveAllowed, loanRepaymentsCount, existsGuarantor);
+                    permissions = this.loanReadPlatformService.retrieveLoanPermissions(loanBasicDetails, isWaiveAllowed,
+                            loanRepaymentsCount, existsGuarantor);
                 }
                 convenienceDataRequired = true;
             }
@@ -230,7 +234,8 @@ public class LoansApiResource {
             if (associationParameters.contains("charges")) {
                 charges = this.chargeReadPlatformService.retrieveLoanCharges(loanId);
                 if (CollectionUtils.isEmpty(charges)) {
-	            	charges = null; // set back to null so doesnt appear in JSON is no charges exist.
+                    charges = null; // set back to null so doesnt appear in JSON
+                                    // is no charges exist.
                 }
             }
         }
@@ -248,7 +253,7 @@ public class LoansApiResource {
         ChargeData chargeTemplate = null;
 
         final boolean template = ApiParameterHelper.template(uriInfo.getQueryParameters());
-		if(template) {
+        if (template) {
             productOptions = this.loanProductReadPlatformService.retrieveAllLoanProductsForLookup();
             loanTermFrequencyTypeOptions = dropdownReadPlatformService.retrieveLoanTermFrequencyTypeOptions();
             repaymentFrequencyTypeOptions = dropdownReadPlatformService.retrieveRepaymentFrequencyTypeOptions();
@@ -265,12 +270,10 @@ public class LoansApiResource {
             chargeTemplate = this.chargeReadPlatformService.retrieveLoanChargeTemplate();
         }
 
-		final LoanAccountData loanAccount = new LoanAccountData(loanBasicDetails, convenienceDataRequired, 
+        final LoanAccountData loanAccount = new LoanAccountData(loanBasicDetails, convenienceDataRequired, repaymentSchedule,
-				repaymentSchedule, loanRepayments, permissions, charges, 
+                loanRepayments, permissions, charges, productOptions, loanTermFrequencyTypeOptions, repaymentFrequencyTypeOptions,
-				productOptions, loanTermFrequencyTypeOptions, repaymentFrequencyTypeOptions, 
+                repaymentStrategyOptions, interestRateFrequencyTypeOptions, amortizationTypeOptions, interestTypeOptions,
-				repaymentStrategyOptions, interestRateFrequencyTypeOptions, 
+                interestCalculationPeriodTypeOptions, fundOptions, chargeOptions, chargeTemplate, null, guarantorData);
-				amortizationTypeOptions, interestTypeOptions, interestCalculationPeriodTypeOptions, 
-				fundOptions, chargeOptions, chargeTemplate, null, guarantorData);
 
         return this.apiJsonSerializerService.serializeLoanAccountDataToJson(prettyPrint, responseParameters, loanAccount);
     }
@@ -280,7 +283,8 @@ public class LoansApiResource {
         if (!CollectionUtils.isEmpty(loanRepayments)) {
             for (LoanTransactionData transaction : loanRepayments) {
                 if (transaction.isNotDisbursement()) {
-					// use this to decide if undo disbural should permission should be set to true.
+                    // use this to decide if undo disbural should permission
+                    // should be set to true.
                     loanRepaymentsCount++;
                 }
             }
@@ -291,10 +295,8 @@ public class LoansApiResource {
     @POST
     @Consumes({ MediaType.APPLICATION_JSON })
     @Produces({ MediaType.APPLICATION_JSON })
-	public String calculateLoanScheduleOrSubmitLoanApplication(
+    public String calculateLoanScheduleOrSubmitLoanApplication(@QueryParam("command") final String commandParam,
-			@QueryParam("command") final String commandParam,
+            @Context final UriInfo uriInfo, final String jsonRequestBody) {
-			@Context final UriInfo uriInfo,
-			final String jsonRequestBody) {
 
         final LoanApplicationCommand command = this.apiDataConversionService.convertJsonToLoanApplicationCommand(null, jsonRequestBody);
 
@@ -320,11 +322,9 @@ public class LoansApiResource {
 
     @PUT
     @Path("{loanId}")
-	@Consumes({MediaType.APPLICATION_JSON})
+    @Consumes({ MediaType.APPLICATION_JSON })
-	@Produces({MediaType.APPLICATION_JSON})
+    @Produces({ MediaType.APPLICATION_JSON })
-	public String modifyLoanApplication(
+    public String modifyLoanApplication(@PathParam("loanId") final Long loanId, final String jsonRequestBody) {
-			@PathParam("loanId") final Long loanId,
-			final String jsonRequestBody) {
 
         final LoanApplicationCommand command = this.apiDataConversionService.convertJsonToLoanApplicationCommand(loanId, jsonRequestBody);
 
@@ -348,8 +348,7 @@ public class LoansApiResource {
     @Path("{loanId}")
     @Consumes({ MediaType.APPLICATION_JSON })
     @Produces({ MediaType.APPLICATION_JSON })
-	public Response stateTransitions(@PathParam("loanId") final Long loanId,
+    public Response stateTransitions(@PathParam("loanId") final Long loanId, @QueryParam("command") final String commandParam,
-			@QueryParam("command") final String commandParam,
             final String jsonRequestBody) {
 
         LoanStateTransitionCommand command = this.apiDataConversionService.convertJsonToLoanStateTransitionCommand(loanId, jsonRequestBody);
@@ -357,55 +356,43 @@ public class LoansApiResource {
         Response response = null;
 
         if (is(commandParam, "reject")) {
-			EntityIdentifier identifier = this.loanWritePlatformService
+            EntityIdentifier identifier = this.loanWritePlatformService.rejectLoan(command);
-					.rejectLoan(command);
             response = Response.ok().entity(identifier).build();
         } else if (is(commandParam, "withdrewbyclient")) {
-			EntityIdentifier identifier = this.loanWritePlatformService
+            EntityIdentifier identifier = this.loanWritePlatformService.withdrawLoan(command);
-					.withdrawLoan(command);
             response = Response.ok().entity(identifier).build();
         } else if (is(commandParam, "approve")) {
-			EntityIdentifier identifier = this.loanWritePlatformService
+            EntityIdentifier identifier = this.loanWritePlatformService.approveLoanApplication(command);
-					.approveLoanApplication(command);
             response = Response.ok().entity(identifier).build();
         } else if (is(commandParam, "disburse")) {
-			EntityIdentifier identifier = this.loanWritePlatformService
+            EntityIdentifier identifier = this.loanWritePlatformService.disburseLoan(command);
-					.disburseLoan(command);
             response = Response.ok().entity(identifier).build();
         }
 
-		UndoStateTransitionCommand undoCommand = new UndoStateTransitionCommand(
+        UndoStateTransitionCommand undoCommand = new UndoStateTransitionCommand(loanId, command.getNote());
-				loanId, command.getNote());
 
         if (is(commandParam, "undoapproval")) {
-			EntityIdentifier identifier = this.loanWritePlatformService
+            EntityIdentifier identifier = this.loanWritePlatformService.undoLoanApproval(undoCommand);
-					.undoLoanApproval(undoCommand);
             response = Response.ok().entity(identifier).build();
         } else if (is(commandParam, "undodisbursal")) {
-			EntityIdentifier identifier = this.loanWritePlatformService
+            EntityIdentifier identifier = this.loanWritePlatformService.undoLoanDisbursal(undoCommand);
-					.undoLoanDisbursal(undoCommand);
             response = Response.ok().entity(identifier).build();
         }
 
-		if (response == null) {
+        if (response == null) { throw new UnrecognizedQueryParamException("command", commandParam); }
-			throw new UnrecognizedQueryParamException("command", commandParam);
-		}
 
         return response;
     }
 
     private boolean is(final String commandParam, final String commandValue) {
-		return StringUtils.isNotBlank(commandParam)
+        return StringUtils.isNotBlank(commandParam) && commandParam.trim().equalsIgnoreCase(commandValue);
-				&& commandParam.trim().equalsIgnoreCase(commandValue);
     }
 
     @POST
     @Path("{loanId}/transactions")
     @Consumes({ MediaType.APPLICATION_JSON })
     @Produces({ MediaType.APPLICATION_JSON })
-	public Response executeLoanTransaction(
+    public Response executeLoanTransaction(@PathParam("loanId") final Long loanId, @QueryParam("command") final String commandParam,
-			@PathParam("loanId") final Long loanId,
-			@QueryParam("command") final String commandParam,
             final String jsonRequestBody) {
 
         final LoanTransactionCommand command = this.apiDataConversionService.convertJsonToLoanTransactionCommand(loanId, jsonRequestBody);
@@ -423,9 +410,7 @@ public class LoansApiResource {
             identifier = this.loanWritePlatformService.closeLoan(command);
         }
 
-		if (identifier == null) {
+        if (identifier == null) { throw new UnrecognizedQueryParamException("command", commandParam); }
-			throw new UnrecognizedQueryParamException("command", commandParam);
-		}
 
         return Response.ok().entity(identifier).build();
     }
@@ -434,9 +419,7 @@ public class LoansApiResource {
     @Path("{loanId}/transactions/template")
     @Consumes({ MediaType.APPLICATION_JSON })
     @Produces({ MediaType.APPLICATION_JSON })
-	public String retrieveNewRepaymentDetails(
+    public String retrieveNewRepaymentDetails(@PathParam("loanId") final Long loanId, @QueryParam("command") final String commandParam,
-			@PathParam("loanId") final Long loanId,
-			@QueryParam("command") final String commandParam,
             @Context final UriInfo uriInfo) {
 
         context.authenticatedUser().validateHasReadPermission("LOAN");
@@ -466,8 +449,7 @@ public class LoansApiResource {
     @Path("{loanId}/transactions/{transactionId}")
     @Consumes({ MediaType.APPLICATION_JSON })
     @Produces({ MediaType.APPLICATION_JSON })
-	public String retrieveTransaction(@PathParam("loanId") final Long loanId,
+    public String retrieveTransaction(@PathParam("loanId") final Long loanId, @PathParam("transactionId") final Long transactionId,
-			@PathParam("transactionId") final Long transactionId,
             @Context final UriInfo uriInfo) {
 
         context.authenticatedUser().validateHasReadPermission("LOAN");
@@ -484,17 +466,13 @@ public class LoansApiResource {
     @Path("{loanId}/transactions/{transactionId}")
     @Consumes({ MediaType.APPLICATION_JSON })
     @Produces({ MediaType.APPLICATION_JSON })
-	public Response adjustLoanTransaction(
+    public Response adjustLoanTransaction(@PathParam("loanId") final Long loanId, @PathParam("transactionId") final Long transactionId,
-			@PathParam("loanId") final Long loanId,
-			@PathParam("transactionId") final Long transactionId,
             final String jsonRequestBody) {
 
-		final AdjustLoanTransactionCommand command = this.apiDataConversionService
+        final AdjustLoanTransactionCommand command = this.apiDataConversionService.convertJsonToAdjustLoanTransactionCommand(loanId,
-				.convertJsonToAdjustLoanTransactionCommand(loanId,
                 transactionId, jsonRequestBody);
 
-		EntityIdentifier identifier = this.loanWritePlatformService
+        EntityIdentifier identifier = this.loanWritePlatformService.adjustLoanTransaction(command);
-				.adjustLoanTransaction(command);
 
         return Response.ok().entity(identifier).build();
     }
@@ -503,9 +481,7 @@ public class LoansApiResource {
     @Path("{loanId}/charges")
     @Consumes({ MediaType.APPLICATION_JSON })
     @Produces({ MediaType.APPLICATION_JSON })
-    public Response addLoanCharge(
+    public Response addLoanCharge(@PathParam("loanId") final Long loanId, final String jsonRequestBody) {
-            @PathParam("loanId") final Long loanId,
-            final String jsonRequestBody){
 
         final LoanChargeCommand command = this.apiDataConversionService.convertJsonToLoanChargeCommand(null, loanId, jsonRequestBody);
 
@@ -516,8 +492,8 @@ public class LoansApiResource {
 
     @GET
     @Path("{loanId}/charges/template")
-    @Consumes({MediaType.APPLICATION_JSON})
+    @Consumes({ MediaType.APPLICATION_JSON })
-    @Produces({MediaType.APPLICATION_JSON})
+    @Produces({ MediaType.APPLICATION_JSON })
     public String retrieveNewLoanChargeDetails(@Context final UriInfo uriInfo) {
 
         context.authenticatedUser().validateHasReadPermission("LOAN");
@@ -534,12 +510,10 @@ public class LoansApiResource {
 
     @GET
     @Path("{loanId}/charges/{chargeId}")
-    @Consumes({MediaType.APPLICATION_JSON})
+    @Consumes({ MediaType.APPLICATION_JSON })
-    @Produces({MediaType.APPLICATION_JSON})
+    @Produces({ MediaType.APPLICATION_JSON })
-    public String retrieveLoanCharge(
+    public String retrieveLoanCharge(@PathParam("loanId") final Long loanId, @PathParam("chargeId") final Long loanChargeId,
-            @PathParam("loanId") final Long loanId,
+            @Context final UriInfo uriInfo) {
-            @PathParam("chargeId") final Long loanChargeId,
-            @Context final UriInfo uriInfo){
 
         context.authenticatedUser().validateHasReadPermission("LOAN");
 
@@ -555,12 +529,11 @@ public class LoansApiResource {
     @Path("{loanId}/charges/{chargeId}")
     @Consumes({ MediaType.APPLICATION_JSON })
     @Produces({ MediaType.APPLICATION_JSON })
-    public String updateLoanCharge(
+    public String updateLoanCharge(@PathParam("loanId") final Long loanId, @PathParam("chargeId") final Long loanChargeId,
-            @PathParam("loanId") final Long loanId,
+            final String jsonRequestBody) {
-            @PathParam("chargeId") final Long loanChargeId,
-            final String jsonRequestBody){
 
-        final LoanChargeCommand command = this.apiDataConversionService.convertJsonToLoanChargeCommand(loanChargeId, loanId, jsonRequestBody);
+        final LoanChargeCommand command = this.apiDataConversionService.convertJsonToLoanChargeCommand(loanChargeId, loanId,
+                jsonRequestBody);
 
         final EntityIdentifier identifier = this.loanWritePlatformService.updateLoanCharge(command);
 
@@ -571,10 +544,8 @@ public class LoansApiResource {
     @Path("{loanId}/charges/{chargeId}")
     @Consumes({ MediaType.APPLICATION_JSON })
     @Produces({ MediaType.APPLICATION_JSON })
-    public String waiveLoanCharge(
+    public String waiveLoanCharge(@PathParam("loanId") final Long loanId, @PathParam("chargeId") final Long loanChargeId,
-            @PathParam("loanId") final Long loanId,
+            @QueryParam("command") final String commandParam) {
-            @PathParam("chargeId") final Long loanChargeId,
-            @QueryParam("command") final String commandParam){
 
         final LoanChargeCommand command = LoanChargeCommand.forWaiver(loanChargeId, loanId);
 
@@ -593,9 +564,7 @@ public class LoansApiResource {
     @Path("{loanId}/charges/{chargeId}")
     @Consumes({ MediaType.APPLICATION_JSON })
     @Produces({ MediaType.APPLICATION_JSON })
-    public String deleteLoanCharge(
+    public String deleteLoanCharge(@PathParam("loanId") final Long loanId, @PathParam("chargeId") final Long loanChargeId) {
-            @PathParam("loanId") final Long loanId,
-            @PathParam("chargeId") final Long loanChargeId){
 
         final EntityIdentifier identifier = this.loanWritePlatformService.deleteLoanCharge(loanId, loanChargeId);
         return this.apiJsonSerializerService.serializeEntityIdentifier(identifier);
@@ -605,9 +574,7 @@ public class LoansApiResource {
     @Path("{loanId}/assign")
     @Consumes({ MediaType.APPLICATION_JSON })
     @Produces({ MediaType.APPLICATION_JSON })
-    public Response assignLoanOfficer(
+    public Response assignLoanOfficer(@PathParam("loanId") final Long loanId, final String jsonRequestBody) {
-            @PathParam("loanId") final Long loanId,
-            final String jsonRequestBody){
 
         final LoanReassignmentCommand command = this.apiDataConversionService.convertJsonToLoanReassignmentCommand(loanId, jsonRequestBody);
 
@@ -620,19 +587,16 @@ public class LoansApiResource {
     @Path("{loanId}/assign/template")
     @Consumes({ MediaType.APPLICATION_JSON })
     @Produces({ MediaType.APPLICATION_JSON })
-    public String assignLoanOfficerTemplate(
+    public String assignLoanOfficerTemplate(@PathParam("loanId") final Long loanId, @Context final UriInfo uriInfo) {
-            @PathParam("loanId") final Long loanId,
-            @Context final UriInfo uriInfo){
 
-        final Set<String> responseParameters = ApiParameterHelper
+        final Set<String> responseParameters = ApiParameterHelper.extractFieldsForResponseIfProvided(uriInfo.getQueryParameters());
-                .extractFieldsForResponseIfProvided(uriInfo
-                        .getQueryParameters());
 
         final boolean prettyPrint = ApiParameterHelper.prettyPrint(uriInfo.getQueryParameters());
 
         LoanBasicDetailsData loanBasicDetails = this.loanReadPlatformService.retrieveLoanAccountDetails(loanId);
 
-        Collection<StaffData> allowedLoanOfficers = this.staffReadPlatformService.retrieveAllLoanOfficersByOffice(loanBasicDetails.getOfficeId());
+        Collection<StaffData> allowedLoanOfficers = this.staffReadPlatformService.retrieveAllLoanOfficersByOffice(loanBasicDetails
+                .getOfficeId());
         Long fromLoanOfficerId = loanBasicDetails.getLoanOfficerId();
 
         LoanReassignmentData loanReassignmentData = LoanReassignmentData.template(fromLoanOfficerId, allowedLoanOfficers, new LocalDate());

mifosng-provider/src/main/java/org/mifosng/platform/api/commands/GuarantorCommand.java

@@ -13,6 +13,7 @@ public class GuarantorCommand {
     private final Long existingClientId;
 
     /*** Fields for external persons serving as guarantors ***/
+    // FIXME - kw - what the details for collecting guarantors change? we are using datatable approach but fixing here?
     private final String firstname;
     private final String lastname;
     private final String addressLine1;
@@ -26,20 +27,16 @@ public class GuarantorCommand {
     private final String comment;
     private final String dob;
 
-	/**Feilds for passing Locale info to generic data service api**/
+    /** Feilds for passing Locale info to generic data service api **/
     private String dateFormat;
     private String locale;
 
     private final Set<String> modifiedParameters;
 
-	public GuarantorCommand(final Set<String> modifiedParameters,
+    public GuarantorCommand(final Set<String> modifiedParameters, final Long existingClientId, final String firstname,
-			final Long existingClientId, final String firstname,
+            final String lastname, final Boolean externalGuarantor, final String addressLine1, final String addressLine2,
-			final String lastname, final Boolean externalGuarantor,
+            final String city, final String state, final String zip, final String country, final String mobileNumber,
-			final String addressLine1, final String addressLine2,
+            final String housePhoneNumber, final String comment, final String dob) {
-			final String city, final String state, final String zip,
-			final String country, final String mobileNumber,
-			final String housePhoneNumber, final String comment,
-			final String dob) {
         this.modifiedParameters = modifiedParameters;
 
         this.externalGuarantor = externalGuarantor;

mifosng-provider/src/main/java/org/mifosng/platform/api/infrastructure/PortfolioApiDataConversionService.java

@@ -95,7 +95,7 @@ public interface PortfolioApiDataConversionService {
 
     CodeCommand convertJsonToCodeCommand(Long resourceIdentifier, String json);
 
-	SavingAccountCommand convertJsonToSavingAccountCommand(Long resourceIdentifier,String json);
+    SavingAccountCommand convertJsonToSavingAccountCommand(Long resourceIdentifier, String json);
 
     GuarantorCommand convertJsonToGuarantorCommand(Long resourceIdentifier, Long loanId, String json);
 }

mifosng-provider/src/main/java/org/mifosng/platform/api/infrastructure/PortfolioApiDataConversionServiceImpl.java

@@ -1631,64 +1631,42 @@ public class PortfolioApiDataConversionServiceImpl implements PortfolioApiDataCo
     }
 
     @Override
-	public GuarantorCommand convertJsonToGuarantorCommand(
+    public GuarantorCommand convertJsonToGuarantorCommand(Long resourceIdentifier, Long loanId, String json) {
-			Long resourceIdentifier, Long loanId, String json) {
+        if (StringUtils.isBlank(json)) { throw new InvalidJsonException(); }
-		if (StringUtils.isBlank(json)) {
-			throw new InvalidJsonException();
-		}
 
-		Type typeOfMap = new TypeToken<Map<String, String>>() {
+        Type typeOfMap = new TypeToken<Map<String, String>>() {}.getType();
-		}.getType();
+        Map<String, String> requestMap = gsonConverter.fromJson(json, typeOfMap);
-		Map<String, String> requestMap = gsonConverter
-				.fromJson(json, typeOfMap);
 
-		Set<String> supportedParams = new HashSet<String>(Arrays.asList(
+        Set<String> supportedParams = new HashSet<String>(Arrays.asList("externalGuarantor", "existingClientId", "firstname", "lastname",
-				"externalGuarantor", "existingClientId", "firstname",
+                "addressLine1", "addressLine2", "city", "state", "zip", "country", "mobileNumber", "housePhoneNumber", "comment", "dob",
-				"lastname", "addressLine1", "addressLine2", "city", "state",
+                "locale", "dateFormat"));
-				"zip", "country", "mobileNumber", "housePhoneNumber",
-				"comment", "dob", "locale", "dateFormat"));
 
         checkForUnsupportedParameters(requestMap, supportedParams);
 
         Set<String> modifiedParameters = new HashSet<String>();
 
-		boolean externalGuarantor = extractBooleanParameter(
+        boolean externalGuarantor = extractBooleanParameter("externalGuarantor", requestMap, modifiedParameters);
-				"externalGuarantor", requestMap, modifiedParameters);
+
+        Long existingClientId = extractLongParameter("existingClientId", requestMap, modifiedParameters);
+        String firstname = extractStringParameter("firstname", requestMap, modifiedParameters);
+        String lastname = extractStringParameter("lastname", requestMap, modifiedParameters);
+        String addressLine1 = extractStringParameter("addressLine1", requestMap, modifiedParameters);
+        String addressLine2 = extractStringParameter("addressLine2", requestMap, modifiedParameters);
+        String city = extractStringParameter("city", requestMap, modifiedParameters);
+        String state = extractStringParameter("state", requestMap, modifiedParameters);
+        String zip = extractStringParameter("zip", requestMap, modifiedParameters);
+        String country = extractStringParameter("country", requestMap, modifiedParameters);
+        String mobileNumber = extractStringParameter("mobileNumber", requestMap, modifiedParameters);
+        String housePhoneNumber = extractStringParameter("housePhoneNumber", requestMap, modifiedParameters);
+        String comment = extractStringParameter("comment", requestMap, modifiedParameters);
+        String dob = extractStringParameter("dob", requestMap, modifiedParameters);
         
-		Long existingClientId = extractLongParameter("existingClientId",
-				requestMap, modifiedParameters);
-		String firstname = extractStringParameter("firstname", requestMap,
-				modifiedParameters);
-		String lastname = extractStringParameter("lastname", requestMap,
-				modifiedParameters);
-		String addressLine1 = extractStringParameter("addressLine1",
-				requestMap, modifiedParameters);
-		String addressLine2 = extractStringParameter("addressLine2",
-				requestMap, modifiedParameters);
-		String city = extractStringParameter("city", requestMap,
-				modifiedParameters);
-		String state = extractStringParameter("state", requestMap,
-				modifiedParameters);
-		String zip = extractStringParameter("zip", requestMap,
-				modifiedParameters);
-		String country = extractStringParameter("country", requestMap,
-				modifiedParameters);
-		String mobileNumber = extractStringParameter("mobileNumber",
-				requestMap, modifiedParameters);
-		String housePhoneNumber = extractStringParameter("housePhoneNumber",
-				requestMap, modifiedParameters);
-		String comment = extractStringParameter("comment", requestMap,
-				modifiedParameters);
-		String dob = extractStringParameter("dob", requestMap,
-				modifiedParameters);
         // workaround for passing locale info to data table api
-		final String dateFormat = (String) requestMap.get("dateFormat");
+        final String dateFormat = requestMap.get("dateFormat");
-		final String locale = (String) requestMap.get("locale");
+        final String locale = requestMap.get("locale");
 
-		GuarantorCommand command = new GuarantorCommand(modifiedParameters,
+        GuarantorCommand command = new GuarantorCommand(modifiedParameters, existingClientId, firstname, lastname, externalGuarantor,
-				existingClientId, firstname, lastname, externalGuarantor,
+                addressLine1, addressLine2, city, state, zip, country, mobileNumber, housePhoneNumber, comment, dob);
-				addressLine1, addressLine2, city, state, zip, country,
-				mobileNumber, housePhoneNumber, comment, dob);
         command.setDateFormat(dateFormat);
         command.setLocale(locale);
         return command;

mifosng-provider/src/main/java/org/mifosng/platform/guarantor/GuarantorReadPlatformServiceImpl.java

@@ -12,39 +12,32 @@ import org.mifosng.platform.exceptions.LoanNotFoundException;
 import org.mifosng.platform.loan.domain.Loan;
 import org.mifosng.platform.loan.domain.LoanRepository;
 import org.mifosng.platform.noncore.ReadWriteNonCoreDataService;
-import org.mifosng.platform.organisation.domain.OfficeRepository;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Service;
 
 @Service
-public class GuarantorReadPlatformServiceImpl implements
+public class GuarantorReadPlatformServiceImpl implements GuarantorReadPlatformService {
-		GuarantorReadPlatformService {
 
-	private final LoanRepository loanRepository;
-	private final ReadWriteNonCoreDataService readWriteNonCoreDataService;
     // Table for storing external Guarantor Details
     public static final String EXTERNAL_GUARANTOR_TABLE_NAME = "m_guarantor_external";
     
+    private final LoanRepository loanRepository;
+    private final ReadWriteNonCoreDataService readWriteNonCoreDataService;
+
     @Autowired
-	public GuarantorReadPlatformServiceImpl(
+    public GuarantorReadPlatformServiceImpl(final LoanRepository loanRepository,
-			final LoanRepository loanRepository,
+            final ReadWriteNonCoreDataService readWriteNonCoreDataService) {
-			final ReadWriteNonCoreDataService readWriteNonCoreDataService,
-			final OfficeRepository officeRepository) {
         this.loanRepository = loanRepository;
         this.readWriteNonCoreDataService = readWriteNonCoreDataService;
     }
 
     @Override
     public boolean existsGuarantor(Long loanId) {
-		Loan loan = validateLoanExists(loanId);
+        Loan loan = retrieveLoanById(loanId);
         // return if internal guarantor exists
-		if (loan.getGuarantor() != null) {
+        if (loan.getGuarantor() != null) { return true; }
-			return true;
-		}
         // return if an external guarantor exists
-		if (null != getExternalGuarantor(loanId)) {
+        if (null != getExternalGuarantor(loanId)) { return true; }
-			return true;
-		}
         // else no guarantor exists
         return false;
     }
@@ -52,48 +45,36 @@ public class GuarantorReadPlatformServiceImpl implements
     @Override
     public GuarantorData retrieveGuarantor(Long loanId) {
         GuarantorData guarantorData = null;
-		Loan loan = validateLoanExists(loanId);
+        Loan loan = retrieveLoanById(loanId);
         // does an internal guarantor exist
         if (loan.getGuarantor() != null) {
             Client guarantor = loan.getGuarantor();
             LocalDate localDate = new LocalDate(guarantor.getJoiningDate());
-			guarantorData = new GuarantorData(guarantor.getId(),
+            guarantorData = new GuarantorData(guarantor.getId(), guarantor.getFirstName(), guarantor.getLastName(),
-					guarantor.getFirstName(), guarantor.getLastName(),
+                    guarantor.getExternalId(), guarantor.getOffice().getName(), localDate);
-					guarantor.getExternalId(), guarantor.getOffice().getName(),
-					localDate);
         } else {
             guarantorData = getExternalGuarantor(loanId);
         }
         // throw error if guarantor does not exist
-		if (guarantorData == null) {
+        if (guarantorData == null) { throw new GuarantorNotFoundException(loanId); }
-			throw new GuarantorNotFoundException(loanId);
-		}
         return guarantorData;
     }
 
-	/**
+    private Loan retrieveLoanById(final Long loanId) {
-	 * @param loanId
+        
-	 * @return
+        // FIXME - KW - On the read side of services - we would prefer not to use the repository/ORM pattern
-	 */
-	private Loan validateLoanExists(Long loanId) {
         Loan loan = loanRepository.findOne(loanId);
-		if (loan == null) {
+        if (loan == null) { throw new LoanNotFoundException(loanId); }
-			throw new LoanNotFoundException(loanId);
-		}
         return loan;
     }
 
-	/**
+    @Override
-	 * @param loanId
+    public GuarantorData getExternalGuarantor(final Long loanId) {
-	 * @return
+        
-	 */
+        GenericResultsetData genericResultDataSet = readWriteNonCoreDataService.retrieveDataTableGenericResultSet(
-	public GuarantorData getExternalGuarantor(Long loanId) {
-		GenericResultsetData genericResultDataSet = readWriteNonCoreDataService
-				.retrieveDataTableGenericResultSet(
                 EXTERNAL_GUARANTOR_TABLE_NAME, loanId, null, null);
         if (genericResultDataSet.getData().size() == 1) {
-			List<String> guarantorRow = genericResultDataSet.getData().get(0)
+            List<String> guarantorRow = genericResultDataSet.getData().get(0).getRow();
-					.getRow();
             String firstname = guarantorRow.get(1);
             String lastname = guarantorRow.get(2);
 
@@ -110,13 +91,10 @@ public class GuarantorReadPlatformServiceImpl implements
             String housePhoneNumber = guarantorRow.get(10);
             String mobileNumber = guarantorRow.get(11);
             String comment = guarantorRow.get(12);
-			GuarantorData guarantorData = new GuarantorData(firstname,
+            GuarantorData guarantorData = new GuarantorData(firstname, lastname, dateOfBirth, addressLine1, addressLine2, city, state, zip,
-					lastname, dateOfBirth, addressLine1, addressLine2, city,
+                    country, mobileNumber, housePhoneNumber, comment);
-					state, zip, country, mobileNumber, housePhoneNumber,
-					comment);
             return guarantorData;
         }
         return null;
     }
-
 }

mifosng-provider/src/main/java/org/mifosng/platform/guarantor/GuarantorWritePlatformServiceImpl.java

@@ -19,28 +19,24 @@ import org.mifosng.platform.exceptions.PlatformApiDataValidationException;
 import org.mifosng.platform.loan.domain.Loan;
 import org.mifosng.platform.loan.domain.LoanRepository;
 import org.mifosng.platform.noncore.ReadWriteNonCoreDataService;
-import org.mifosng.platform.organisation.domain.OfficeRepository;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Service;
 import org.springframework.transaction.annotation.Transactional;
 
 @Service
-public class GuarantorWritePlatformServiceImpl implements
+public class GuarantorWritePlatformServiceImpl implements GuarantorWritePlatformService {
-		GuarantorWritePlatformService {
+
+    // Table for storing external Guarantor Details
+    public static final String EXTERNAL_GUARANTOR_TABLE_NAME = "m_guarantor_external";
 
     private final LoanRepository loanRepository;
     private final ClientRepository clientRepository;
     private final ReadWriteNonCoreDataService readWriteNonCoreDataService;
     private final GuarantorReadPlatformService guarantorReadPlatformService;
-	// Table for storing external Guarantor Details
-	public static final String EXTERNAL_GUARANTOR_TABLE_NAME = "m_guarantor_external";
 
     @Autowired
-	public GuarantorWritePlatformServiceImpl(
+    public GuarantorWritePlatformServiceImpl(final LoanRepository loanRepository,
-			final LoanRepository loanRepository,
+            final ReadWriteNonCoreDataService readWriteNonCoreDataService, final ClientRepository clientRepository,
-			final ReadWriteNonCoreDataService readWriteNonCoreDataService,
-			final OfficeRepository officeRepository,
-			final ClientRepository clientRepository,
             final GuarantorReadPlatformService guarantorReadPlatformService) {
         this.loanRepository = loanRepository;
         this.readWriteNonCoreDataService = readWriteNonCoreDataService;
@@ -50,100 +46,76 @@ public class GuarantorWritePlatformServiceImpl implements
 
     @Override
     @Transactional
-	public void createGuarantor(Long loanId, GuarantorCommand command) {
+    public void createGuarantor(final Long loanId, final GuarantorCommand command) {
-		GuarantorCommandValidator validator = new GuarantorCommandValidator(
+        GuarantorCommandValidator validator = new GuarantorCommandValidator(command);
-				command);
         validator.validateForCreate();
         saveOrUpdateGuarantor(loanId, command);
     }
 
     @Override
     @Transactional
-	public void updateGuarantor(Long loanId, GuarantorCommand command) {
+    public void updateGuarantor(final Long loanId, final GuarantorCommand command) {
-		GuarantorCommandValidator validator = new GuarantorCommandValidator(
+        GuarantorCommandValidator validator = new GuarantorCommandValidator(command);
-				command);
         validator.validateForUpdate();
         saveOrUpdateGuarantor(loanId, command);
     }
 
     @Override
     @Transactional
-	public void removeGuarantor(Long loanId) {
+    public void removeGuarantor(final Long loanId) {
-		Loan loan = validateLoanExists(loanId);
+        Loan loan = retrieveLoanById(loanId);
         // remove internal guarantor, if any
         if (loan.getGuarantor() != null) {
             loan.setGuarantor(null);
             loanRepository.saveAndFlush(loan);
         }
         // remove external guarantor, if any
-		readWriteNonCoreDataService.deleteDatatableEntries(
+        readWriteNonCoreDataService.deleteDatatableEntries(EXTERNAL_GUARANTOR_TABLE_NAME, loanId);
-				EXTERNAL_GUARANTOR_TABLE_NAME, loanId);
     }
 
-	/**
+    private void saveOrUpdateGuarantor(final Long loanId, final GuarantorCommand command) {
-	 * @param loanId
+        Loan loan = retrieveLoanById(loanId);
-	 * @param command
-	 */
-	private void saveOrUpdateGuarantor(Long loanId, GuarantorCommand command) {
-		Loan loan = validateLoanExists(loanId);
         // mark an existing client as a guarantor
-		if (command.isExternalGuarantor() == null
+        if (command.isExternalGuarantor() == null || !command.isExternalGuarantor()) {
-				|| !command.isExternalGuarantor()) {
             // can't set client as a guarantor to himself
-			if (loan.getClient().getId().equals(command.getExistingClientId())) {
+            if (loan.getClient().getId().equals(command.getExistingClientId())) { throw new InvalidGuarantorException(
-				throw new InvalidGuarantorException(
+                    command.getExistingClientId(), loanId); }
-						command.getExistingClientId(), loanId);
+            Client guarantor = clientRepository.findOne(command.getExistingClientId());
-			}
+            if (guarantor == null) { throw new ClientNotFoundException(command.getExistingClientId()); }
-			Client guarantor = clientRepository.findOne(command
-					.getExistingClientId());
-			if (guarantor == null) {
-				throw new ClientNotFoundException(command.getExistingClientId());
-			}
             loan.setGuarantor(guarantor);
             this.loanRepository.saveAndFlush(loan);
             // also delete any existing External Guarantors
-			readWriteNonCoreDataService.deleteDatatableEntries(
+            readWriteNonCoreDataService.deleteDatatableEntries(EXTERNAL_GUARANTOR_TABLE_NAME, loanId);
-					EXTERNAL_GUARANTOR_TABLE_NAME, loanId);
         }// or create an external guarantor
         else {
             Set<String> modifiedParameters = command.getModifiedParameters();
             Map<String, String> modifiedParametersMap = new HashMap<String, String>();
+            
             /***
              * TODO Vishwas: Check with JW/KW if using reflection here is a good
              * idea
              **/
-			Class<? extends GuarantorCommand> guarantorCommandClass = command
+            Class<? extends GuarantorCommand> guarantorCommandClass = command.getClass();
-					.getClass();
             for (String modifiedParameter : modifiedParameters) {
                 Method method;
                 try {
-					if (modifiedParameter.equalsIgnoreCase("externalGuarantor")
+                    if (modifiedParameter.equalsIgnoreCase("externalGuarantor") || modifiedParameter.equalsIgnoreCase("existingClientId")) {
-							|| modifiedParameter
-									.equalsIgnoreCase("existingClientId")) {
                         continue;
                     } else {
-						method = guarantorCommandClass.getMethod("get"
+                        method = guarantorCommandClass.getMethod("get" + StringUtils.capitalize(modifiedParameter));
-								+ StringUtils.capitalize(modifiedParameter));
                     }
-					String columnName = StringUtils.join(StringUtils
+                    String columnName = StringUtils.join(StringUtils.splitByCharacterTypeCamelCase(modifiedParameter), '_');
-							.splitByCharacterTypeCamelCase(modifiedParameter),
+                    modifiedParametersMap.put(columnName.toLowerCase(), method.invoke(command).toString());
-							'_');
-					modifiedParametersMap.put(columnName.toLowerCase(), method
-							.invoke(command).toString());
                 } catch (Exception e) {
                     // TODO: This block would ideally never be reached, could
                     // use an
                     // empty catch block instead?
                     List<ApiParameterError> dataValidationErrors = new ArrayList<ApiParameterError>();
-					ApiParameterError apiParameterError = ApiParameterError
+                    ApiParameterError apiParameterError = ApiParameterError.parameterError("validation.msg.validation.errors.exist", "",
-							.parameterError(
+                            modifiedParameter);
-									"validation.msg.validation.errors.exist",
-									"", modifiedParameter);
                     dataValidationErrors.add(apiParameterError);
-					throw new PlatformApiDataValidationException(
+                    throw new PlatformApiDataValidationException("validation.msg.validation.errors.exist",
-							"validation.msg.validation.errors.exist",
+                            "Invalid parameter passed for Creating updating Guarantor", dataValidationErrors);
-							"Invalid parameter passed for Creating updating Guarantor",
-							dataValidationErrors);
                 }
 
             }
@@ -152,13 +124,9 @@ public class GuarantorWritePlatformServiceImpl implements
             modifiedParametersMap.put("locale", command.getLocale());
             modifiedParametersMap.put("dateFormat", command.getDateFormat());
             if (guarantorReadPlatformService.getExternalGuarantor(loanId) != null) {
-				readWriteNonCoreDataService.updateDatatableEntryOnetoOne(
+                readWriteNonCoreDataService.updateDatatableEntryOnetoOne(EXTERNAL_GUARANTOR_TABLE_NAME, loanId, modifiedParametersMap);
-						EXTERNAL_GUARANTOR_TABLE_NAME, loanId,
-						modifiedParametersMap);
             } else {
-				readWriteNonCoreDataService.newDatatableEntry(
+                readWriteNonCoreDataService.newDatatableEntry(EXTERNAL_GUARANTOR_TABLE_NAME, loanId, modifiedParametersMap);
-						EXTERNAL_GUARANTOR_TABLE_NAME, loanId,
-						modifiedParametersMap);
             }
             // finally unset any existing internal guarantors
             if (loan.getGuarantor() != null) {
@@ -168,15 +136,9 @@ public class GuarantorWritePlatformServiceImpl implements
         }
     }
 
-	/**
+    private Loan retrieveLoanById(final Long loanId) {
-	 * @param loanId
-	 * @return
-	 */
-	private Loan validateLoanExists(Long loanId) {
         Loan loan = loanRepository.findOne(loanId);
-		if (loan == null) {
+        if (loan == null) { throw new LoanNotFoundException(loanId); }
-			throw new LoanNotFoundException(loanId);
-		}
         return loan;
     }
 }