snippetjavaMinor
How can I reduce the number of nested if statements in this code?
Viewed 0 times
thiscannumberthestatementsnestedreducehowcode
Problem
The application I'm building can accept two types of updates: Application Update and Parameter Updates. If there is an application update, then parameter updates can be ignored. This is the code that I've written, and it's messy. How can I clean this up?
```
mContext = this.getApplicationContext();
//experimental();
//check for updates
UpdateMode updateInstallationMode = Parameters.load(mContext).getUpdateMode();
if(updateInstallationMode==UpdateMode.MANUAL)
{
//User will manually install update - no further action needed to be performed by activity.
//proceed to next screen.
gotoNextActivity();
}else
{
boolean shouldConfirm = (updateInstallationMode == UpdateMode.CONFIRM);
lblStatus.setText(getString(R.string.checking_for_updates));
Error outUpdateError = new Error();
Map availableUpdates = checkForUpdates(outUpdateError);
boolean appUpdatesAvailable = availableUpdates.containsKey("application");
boolean paramUpdatesAvailable = availableUpdates.containsKey("parameters");
boolean appUpdatesAccepted = Parameters.load(mContext).getUpdateType()==UpdateType.BOTH
|| Parameters.load(mContext).getUpdateType()==UpdateType.APPLICATION;
boolean paramUpdatesAccepted = Parameters.load(mContext).getUpdateType() == UpdateType.BOTH
|| Parameters.load(mContext).getUpdateType() == UpdateType.PARAMETERS;
if(appUpdatesAccepted && appUpdatesAvailable){
boolean shouldInstall = shouldConfirm? confirmInstallUpdate(UpdateType.APPLICATION):true;
if(shouldInstall){
downloadUpdate(UpdateType.APPLICATION,availableUpdates.get("application"));
}else{
gotoNextActivity();
}
}else if(paramUpdatesAccepted && paramUpdatesAvailable){
boolean shouldInstall = shouldConfirm? confirmInstallUpdate(UpdateType.PARAMETERS):true;
if(shouldInstall){
downloadUpdate(UpdateType.PARAMETERS,availableUpdates.get("parameters"));
}else{
```
mContext = this.getApplicationContext();
//experimental();
//check for updates
UpdateMode updateInstallationMode = Parameters.load(mContext).getUpdateMode();
if(updateInstallationMode==UpdateMode.MANUAL)
{
//User will manually install update - no further action needed to be performed by activity.
//proceed to next screen.
gotoNextActivity();
}else
{
boolean shouldConfirm = (updateInstallationMode == UpdateMode.CONFIRM);
lblStatus.setText(getString(R.string.checking_for_updates));
Error outUpdateError = new Error();
Map availableUpdates = checkForUpdates(outUpdateError);
boolean appUpdatesAvailable = availableUpdates.containsKey("application");
boolean paramUpdatesAvailable = availableUpdates.containsKey("parameters");
boolean appUpdatesAccepted = Parameters.load(mContext).getUpdateType()==UpdateType.BOTH
|| Parameters.load(mContext).getUpdateType()==UpdateType.APPLICATION;
boolean paramUpdatesAccepted = Parameters.load(mContext).getUpdateType() == UpdateType.BOTH
|| Parameters.load(mContext).getUpdateType() == UpdateType.PARAMETERS;
if(appUpdatesAccepted && appUpdatesAvailable){
boolean shouldInstall = shouldConfirm? confirmInstallUpdate(UpdateType.APPLICATION):true;
if(shouldInstall){
downloadUpdate(UpdateType.APPLICATION,availableUpdates.get("application"));
}else{
gotoNextActivity();
}
}else if(paramUpdatesAccepted && paramUpdatesAvailable){
boolean shouldInstall = shouldConfirm? confirmInstallUpdate(UpdateType.PARAMETERS):true;
if(shouldInstall){
downloadUpdate(UpdateType.PARAMETERS,availableUpdates.get("parameters"));
}else{
Solution
I assume that the code in the question is a method. If not extract it out to a method.
-
I'd start with removing some duplication here.
I'm not familiar with Android, so
-
If you reorder the
This could be extracted out to a method:
Usage:
-
It's also possible to extract out the first two
Usage:
-
I'd move the
```
public void update() {
doUpdate();
gotoNextActivity();
}
private void doUpdate() {
mContext = this.getApplicationContext();
final UpdateParameters updateParameters = Parameters.load(mContext);
final UpdateMode updateInstallationMode =
updateParameters.getUpdateMode();
if (updateInstallationMode == UpdateMode.MANUAL) {
return;
}
lblStatus.setText(getString(RString.checking_for_updates));
final Error outUpdateError = new Error();
final Map availableUpdates =
checkForUpdates(outUpdateError);
final boolean tryApplicationUpdate =
isUpdateAvailable(availableUpdates, updateParameters,
UpdateType.APPLICATION, "application");
final boolean tryParametersUpdate =
isUpdateAvailable(availableUpdates, updateParameters,
UpdateType.PARAMETERS, "parameters");
if (tryApplicationUpdate) {
downloadUpdateIfConfirmed(updateInstallationMode,
availableUpdates, UpdateType.APPLICATION, "application");
return;
}
if (tryParametersUpdate) {
downloadUpdateIfConfirmed(updateInstallationMode,
availableUpdates, UpdateType.PARAMETERS, "parameters");
return;
}
lblStatus.setText(getString(RString.app_up_to_date));
}
private void downloadUpdateIfConfirmed(
final UpdateMode updateInstallationMode,
final Map availableUpd
-
boolean appUpdatesAccepted = Parameters.load(mContext).getUpdateType() == UpdateType.BOTH
|| Parameters.load(mContext).getUpdateType() == UpdateType.APPLICATION;
boolean paramUpdatesAccepted = Parameters.load(mContext).getUpdateType() == UpdateType.BOTH
|| Parameters.load(mContext).getUpdateType() == UpdateType.PARAMETERS;I'd start with removing some duplication here.
UpdateParameters updateParameters = Parameters.load(mContext);
...
final UpdateType updateType = updateParameters.getUpdateType();
boolean appUpdatesAccepted = updateType == UpdateType.BOTH
|| updateType == UpdateType.APPLICATION;
boolean paramUpdatesAccepted = updateType == UpdateType.BOTH
|| updateType == UpdateType.PARAMETERS;I'm not familiar with Android, so
UpdateParameters and UpdateType types could be something else.-
If you reorder the
boolean variables you can find some more duplication:final boolean appUpdatesAvailable = availableUpdates
.containsKey("application");
final boolean appUpdatesAccepted = updateType == UpdateType.BOTH
|| updateType == UpdateType.APPLICATION;
final boolean paramUpdatesAvailable = availableUpdates
.containsKey("parameters");
final boolean paramUpdatesAccepted = updateType == UpdateType.BOTH
|| updateType == UpdateType.PARAMETERS;This could be extracted out to a method:
private boolean isUpdateAvailable(
final Map availableUpdates,
final UpdateParameters updateParameters,
final UpdateType updateType, final String updateKey) {
final UpdateType availableUpdateType = updateParameters
.getUpdateType();
final boolean appUpdatesAvailable = availableUpdates
.containsKey(updateKey);
final boolean appUpdatesAccepted =
availableUpdateType == UpdateType.BOTH
|| availableUpdateType == updateType;
return appUpdatesAvailable && appUpdatesAccepted;
}Usage:
final boolean tryApplicationUpdate =
isUpdateAvailable(availableUpdates, updateParameters,
UpdateType.APPLICATION, "application");
final boolean tryParametersUpdate =
isUpdateAvailable(availableUpdates, updateParameters,
UpdateType.PARAMETERS, "parameters");
if (tryApplicationUpdate) {
...
} else if (tryParametersUpdate) {
...
} else {
...
}-
It's also possible to extract out the first two
if blocks to a method, they are very similar to each other:private void downloadUpdateIfConfirmed(
final boolean shouldConfirm,
final Map availableUpdates,
final UpdateType currentUpdateType, final String updateKey) {
final boolean shouldInstall = shouldConfirm ?
confirmInstallUpdate(currentUpdateType) : true;
if (shouldInstall) {
downloadUpdate(currentUpdateType, availableUpdates.get(updateKey));
} else {
gotoNextActivity();
}
}Usage:
if (tryApplicationUpdate) {
downloadUpdateIfConfirmed(shouldConfirm, availableUpdates,
UpdateType.APPLICATION, "application");
} else if (tryParametersUpdate) {
downloadUpdateIfConfirmed(shouldConfirm, availableUpdates,
UpdateType.PARAMETERS, "parameters");
} else {
...
}-
I'd move the
shouldConfirm flag into the method (to reduce its scope, see Effective Java, Second Edition, Item 45: Minimize the scope of local variables); use guard clauses; create another method which calls gotoNextActivity() to remove its duplication:```
public void update() {
doUpdate();
gotoNextActivity();
}
private void doUpdate() {
mContext = this.getApplicationContext();
final UpdateParameters updateParameters = Parameters.load(mContext);
final UpdateMode updateInstallationMode =
updateParameters.getUpdateMode();
if (updateInstallationMode == UpdateMode.MANUAL) {
return;
}
lblStatus.setText(getString(RString.checking_for_updates));
final Error outUpdateError = new Error();
final Map availableUpdates =
checkForUpdates(outUpdateError);
final boolean tryApplicationUpdate =
isUpdateAvailable(availableUpdates, updateParameters,
UpdateType.APPLICATION, "application");
final boolean tryParametersUpdate =
isUpdateAvailable(availableUpdates, updateParameters,
UpdateType.PARAMETERS, "parameters");
if (tryApplicationUpdate) {
downloadUpdateIfConfirmed(updateInstallationMode,
availableUpdates, UpdateType.APPLICATION, "application");
return;
}
if (tryParametersUpdate) {
downloadUpdateIfConfirmed(updateInstallationMode,
availableUpdates, UpdateType.PARAMETERS, "parameters");
return;
}
lblStatus.setText(getString(RString.app_up_to_date));
}
private void downloadUpdateIfConfirmed(
final UpdateMode updateInstallationMode,
final Map availableUpd
Code Snippets
boolean appUpdatesAccepted = Parameters.load(mContext).getUpdateType() == UpdateType.BOTH
|| Parameters.load(mContext).getUpdateType() == UpdateType.APPLICATION;
boolean paramUpdatesAccepted = Parameters.load(mContext).getUpdateType() == UpdateType.BOTH
|| Parameters.load(mContext).getUpdateType() == UpdateType.PARAMETERS;UpdateParameters updateParameters = Parameters.load(mContext);
...
final UpdateType updateType = updateParameters.getUpdateType();
boolean appUpdatesAccepted = updateType == UpdateType.BOTH
|| updateType == UpdateType.APPLICATION;
boolean paramUpdatesAccepted = updateType == UpdateType.BOTH
|| updateType == UpdateType.PARAMETERS;final boolean appUpdatesAvailable = availableUpdates
.containsKey("application");
final boolean appUpdatesAccepted = updateType == UpdateType.BOTH
|| updateType == UpdateType.APPLICATION;
final boolean paramUpdatesAvailable = availableUpdates
.containsKey("parameters");
final boolean paramUpdatesAccepted = updateType == UpdateType.BOTH
|| updateType == UpdateType.PARAMETERS;private boolean isUpdateAvailable(
final Map<String, String> availableUpdates,
final UpdateParameters updateParameters,
final UpdateType updateType, final String updateKey) {
final UpdateType availableUpdateType = updateParameters
.getUpdateType();
final boolean appUpdatesAvailable = availableUpdates
.containsKey(updateKey);
final boolean appUpdatesAccepted =
availableUpdateType == UpdateType.BOTH
|| availableUpdateType == updateType;
return appUpdatesAvailable && appUpdatesAccepted;
}final boolean tryApplicationUpdate =
isUpdateAvailable(availableUpdates, updateParameters,
UpdateType.APPLICATION, "application");
final boolean tryParametersUpdate =
isUpdateAvailable(availableUpdates, updateParameters,
UpdateType.PARAMETERS, "parameters");
if (tryApplicationUpdate) {
...
} else if (tryParametersUpdate) {
...
} else {
...
}Context
StackExchange Code Review Q#41780, answer score: 5
Revisions (0)
No revisions yet.