patternjavaMinor
Querying the Google ads API
Viewed 0 times
thegooglequeryingapiads
Problem
I have a method which relies heavily on object supplied by third party APIs. Below is my working code, is there any scope of improvement?
Every type used above belongs to third party code. I also need to know the good strategy to handle exceptions?
Update 1
One frequent issue I encounter and I am sure many if us will is that there is usually references to the third party type
Reference code here.
public LineItem getLineItem(
String networkId, String lineItemId) throws ApiException_Exception {
LineItem lineItem = null;
session.setCode(networkId);
LineItemServiceInterface lineItemService = servicesInterface.lineItemService(dfpSession);
StatementBuilder statementBuilder =
new StatementBuilder()
.where("id = " + lineItemId.trim())
.orderBy("id ASC")
.limit(StatementBuilder.SUGGESTED_PAGE_LIMIT);
LineItemPage lineItemPage =
lineItemService.getLineItemsByStatement(statementBuilder.toStatement());
if (lineItemPage != null && lineItemPage.getResults() != null) {
lineItem = lineItemPage.getResults().get(0);
}
return lineItem;
}Every type used above belongs to third party code. I also need to know the good strategy to handle exceptions?
Update 1
One frequent issue I encounter and I am sure many if us will is that there is usually references to the third party type
LineItem is sprinkled everywhere in the code base. Is this a good practice to leave it like that or should I wrap that type into my own type and use that instead?Reference code here.
Solution
SQL statement construction
Your third-party library do support bindings, so instead of
Early variable assignment
You usually should not need to declare the return result right at the start, perform the optional assignment, and then return from the method. You should just return below, without needing the
Your third-party library do support bindings, so instead of
.where("id = " + lineItemId.trim()), you should consider:// .where("id = " + lineItemId.trim())
.where("id = :lineItem")
.withBindVariableValue("lineItem", lineItemId.trim());Early variable assignment
LineItem lineItem = null;
// ...
if (/* ... */) {
lineItem = /* ... */;
}
return lineItem;You usually should not need to declare the return result right at the start, perform the optional assignment, and then return from the method. You should just return below, without needing the
lineItem variable:// ...
if (/* ... */ ) {
return /* ... */;
}
return null;Code Snippets
// .where("id = " + lineItemId.trim())
.where("id = :lineItem")
.withBindVariableValue("lineItem", lineItemId.trim());LineItem lineItem = null;
// ...
if (/* ... */) {
lineItem = /* ... */;
}
return lineItem;// ...
if (/* ... */ ) {
return /* ... */;
}
return null;Context
StackExchange Code Review Q#122451, answer score: 4
Revisions (0)
No revisions yet.