Java Practice in Practice - Returns mutliple values from a method
I have two questions about the Java Convention. I try to use Robert K. Martin's "Clean Code" code.
Next case:
public void startProgressIfAllowed() {
try {
tryStartProgressIfAllowed();
} catch (Exception exception) {
// log error
}
}
private void tryStartProgressIfAllowed() {
if (isStartProgressAllowed()) {
stopProgressOnCurrentlyStartedTask();
startProgressOnThisTask();
}
}
private boolean isStartProgressAllowed() {
// Calls JOptionPane.showConfirmDialog with JOptionPane.YES_NO_OPTION.
// Created dialog contains checkbox indicating that saving currently started task is required.
// returns boolean depending on JOptionPane.YES_NO_OPTION clicked button
}
private void stopProgressOnCurrentlyStartedTask() {
// Saves currently started task depending on checkbox selecion property and stops currently started.
// What is the correct way to get checkbox selecion property?
}
Suggested solution:
public void tryStartProgressIfAllowed() {
if (tryToStopProgressOnStartedTaskIfNecessary()) {
startProgressOnThisTask();
}
}
private boolean tryToStopProgressOnStartedTaskIfNecessary() {
// Calls JOptionPane.showConfirmDialog with JOptionPane.YES_NO_OPTION.
// Created dialog contains checkbox indicating that saving currently started task is required.
// Depending on checkbox selecion property saves task.
// returns boolean depending on JOptionPane.YES_NO_OPTION clicked button
}
- But this approach does not follow the "Command Query Separation" principle, because the tryToStopProgressOnStartedTaskIfNecessary (...) method does some logic and returns a success / failure value.
- I think that this approach also does not follow the principle of "One level of abstraction per function" because I believe that the operations "check" and "save" are at different levels of abstraction.
- Is the method name correct to avoid misinformation? Perhaps a better name would be tryToStopProgressAndSaveStartedTaskIfNecessary (...)?
Is there a better solution for the above problem?
source to share
How about the following:
public void tryStartProgressOnThisTaskIfAllowed() {
tryStopTaskInProgressIfAllowed()
if (!isTaskInProgress()) {
tryStartProgressOnThisTask();
}
}
private void tryStopTaskInProgressIfAllowed() {
if (!isTaskInProgress()) {
return;
}
TaskInProgressResult result = whatToDoWithTaskInProgress();
if (result == Result.KEEP) {
return;
} else if (result == Result.DROP)
tryDropTaskInProgress();
} else if (result == Result.SAVE) {
trySaveTaskInProgress();
}
}
About your points:
- You now have two separate methods for C and Q
- I think two things
whatToDoWithTaskInProgress
andtryDropTaskInProgress
are the same level of abstraction. If you had entered the code of one or the other, you were absolutely right, of course. - I changed some of the method names to suit my taste :) The only thing I still don't like is the "OnThisTask" part, because this task is somewhat pointless. Maybe it's just because the rest of the code is unknown, maybe OnNextTask or OnNewTask is better.
The problem we ran into is that we were thinking about the value of the YES / NO + checkbox in the UI. But here it is much better to think in business matters. I have highlighted three different results that are of interest: KEEP, SAVE, DROP. How the answer is obtained is irrelevant to the calling method.
source to share
Seems to ask something in CodeReview, see the dropdown at the top left of the page.
An example of how such publicity is implemented in Java SE is the regular expression Matcher class.
String s = ...
Pattern pattern = Pattern.compile("...");
Matcher m = pattern.matcher(s);
StringBuffer sb = new StringBuffer();
while (m.find()) {
m.appendReplacement(sb, ... m.group(1) ...);
}
m.appendTail(sb);
with m.matches()
and m.lookingAt
as alternative schemes.
In a short state, the actual data is stored in the processing class (String here).
source to share