Can I avoid repeating myself in this situation (Java)

Question

 if (openFile == null) {

      new AppFileDialog().chooseFile("Save", appFrame);

 }

 if (openFile == null) {

      return;

 }

Here I need to check to see if the user has already chosen a file. If not, they are given a prompt to. If the file is still null, the function returns without saving. The problem is the two identical if statements, can I avoid it? I take DRY very seriously, but at the same time KISS. Ideally the two go hand in hand, but in a situation like this, it seems they are mutually exclusive.

Answer 1

Put it in a loop? The file the user selects should never be null though

However you've cut out too much code give any concrete answer. All I see is two identical checks which I would merge into one, but I think you coming here for something more.

Answer 2

Not quite, although I think a different structure would make the issue more obvious:

// If no file, give the user a chance to open one
if (openFile == null) {
    new AppFileDialog().chooseFile("Save", appFrame);

    // still no file, user must not want to do this
    if (openFile == null) {
        return;
    }    
}

Answer 3

I would try to get rid of that side effect (setting openFile in the chooseFile method), because it makes the code hard to follow. Can't you return it?

It will not solve the double null check though.

Answer 4

They are actually different conditions. What I think you really mean is:

if (openFile == null) {
    openFile = new AppFileDialog().chooseFile("Save", appFrame);

    if (openFile == null) {
        return;
    }
}

Which shows that they don't mean the same thing, but yours is more elegant if you wish to add more conditions (more ways that the file could be opened, like using a default filename if the user doesn't supply one himself)

However I'd prefer:

openFile = getOpenFile()
if(openFile == null)
    return;

public File getOpenFile() {
   if(openFile == null)
       openFile = new AppFileDialog().chooseFile("Save", appFrame);

   return openFile;
}

This allows the getOpenFile() method to control the openFile variable completely, never accessing the openFile variable from any other methods (except perhaps a closeFile() method. I use this trick sometimes, making a variable that is "Logically private" to just a couple methods in order to reduce complexity a little.

Answer 5

You may try something like this, but this assumes that chooseFile will return the file.

if ((openFile == null ? new AppFileDialog().chooseFile("Save", appFrame) : openFile) == null) 
  return;

Answer 6

Ill do something like:

int tries = 0;
int maxTries = 3;
do {
   openFile = new AppFileDialog().chooseFile("Save", appFrame);
   if (openFile != null) 
      tries = maxTries;
   tries++;
} while (tries < maxTries);

if (openFile == null)
   return;