FlowScene::load() calls clearScene() before a file is chosen or operation is cancelled

[devlpr]

In the case where a person wants to load a file and then decides not to, the scene should not clear. The clearScene() call should move down toward the bottom after the file is checked and opened and the user hasn't cancelled. Clearly a person can write their own save routine but since one is here it should probably be well behaved.

`void FlowScene:: load() { QString fileName = QFileDialog::getOpenFileName(nullptr, tr("Open Flow Scene"), QDir::homePath(), tr("Flow Scene Files (*.flow)"));

if (!QFileInfo::exists(fileName)) return;

QFile file(fileName);

if (!file.open(QIODevice::ReadOnly)) return;

QByteArray wholeFile = file.readAll(); clearScene(); <----------- HERE loadFromMemory(wholeFile); } `

[Daguerreo]

Yep. changing the last part with

      if(file.open(QIODevice::ReadOnly)){
         clearScene();
         loadFromMemory(file.readAll());
      }

would easily fix it, though I would add some check and at least return a bool for sucessful operation or not

[devlpr]

The bool wouldn't be useful in my use-case but it can't hurt. Maybe somebody else needs it.

It looks like readAll() will never throw, so what you are doing is good. I initially had the clearScene() after the readAll() in case readAll() throws an exception. I just read the docs for it and it doesn't throw. So what you are doing is better.

[paceholder]

Maybe I did not understood it well, but what is the principle difference between this

  if (!file.open(QIODevice::ReadOnly))
    return;

  QByteArray wholeFile = file.readAll();
  clearScene();                                           <----------- HERE
  loadFromMemory(wholeFile);

and this

      if(file.open(QIODevice::ReadOnly)){
         clearScene();
         loadFromMemory(file.readAll());
      }

Currently in the master if a file dialog was closed with "Cancel" , the file is not opened and the scene remains intact

[Daguerreo]

Since years passed I don't remember the whole flow, but I remember there was a scenario where the scene were erased and a new scene wasn't load. I'd suggest to just close the issue, if something similar appears again we can just notify a new one :)

[devlpr]

Seems reasonable.