Andrew,
Yes, it could still occur, but the point is that loadInput is called by other places in ICE and not intended to be called the way you were doing it. Specifically, loadInput should never be called from setupForm because it called after construction when the
user specifies a file. There are situations where I could overwrite your Form with junk. For example, in the case with the Proteus and Nek5000 models, some services were not allocated yet and it barfed.
So, the errors could still happen, but we will know exactly where and why.
The final issue here is one of code cleanliness. In general, when you are designing something, put things that perform different tasks into different functions until you have three or more examples that you can use to generalize (via induction) a single
function. That does not mean that you take the code from each one and put it into a switch like "if (f != null) ... else ..." and call one function, but that you write one function with minimal branching that does the work of all three originals. (See our
developer guide for why I hate switches.) In this case, that means we have loadDefaults() and loadInput(), which we will merge and chain in the future when we change the backend persistence model and construction scheme for the Items.
Jay
Jay Jay Billings
Oak Ridge National Laboratory
Twitter Handle: @jayjaybillings
From: ice-dev-bounces@xxxxxxxxxxx <ice-dev-bounces@xxxxxxxxxxx> on behalf of Andrew Bennett <bennett.andr@xxxxxxxxx>
Sent: Wednesday, July 13, 2016 2:19 PM
To: ice developer discussions
Subject: Re: [ice-dev] Handling loadInput in newly created items for tutorial
Sure we can do that. I'm a little confused about how that alleviates the possibility of bugs though. In the issue you wrote:
> The problem with setupForm() being part of the constructor means that the loadInput operation could be, but might not always be, called before the form is initialized properly or even before the project is properly configured.
Here's what confuses me. In the old code setupForm did (essentially) this:
form = new Form();
ioService = getIOService();
loadInput(null);
And the new code is going to be:
form = new Form();
ioService = getIOService();
loadDefaults();
Where loadDefaults is going to just be the code from loadInput in the if(fileName == null) block. Couldn't the situation outlined in your comment still occur with the
new code?
Andrew
|