Angus Leeming wrote: > Looks good except for this: > >> Index: frontends/Dialogs.C >> Dialog * Dialogs::find(string const & name) >> { >> - if (!isValidName(name)) >> - return 0; >> - >> std::map<string, DialogPtr>::iterator it = >> dialogs_.find(name); >> >> if (it == dialogs_.end()) { >> + if (!isValidName(name)) >> + return 0; >> dialogs_[name] = DialogPtr(build(name)); >> return dialogs_[name].get(); >> } > > If the name isn't valid it's because the dialog isn't implemented in that > frontend. It should be the first test therefore. > > ps, as of tomorrow am I'm away till Monday. >
I don't agree... The name is likely to be always valid (except for an incomplete frontend, that shouldn't be used anyway). So this change will tipically avoid an unecesary check and speed up things when navigating with a dialog opened... Of course, it doesn't make such a huge difference. I will apply without it (but please reconsider). Regards, Alfredo