Patrick Lam wrote: > > fix bug 2746 caused by poor parameter naming in pd_Document::_syncFileTypes [...] > Hint: calling variables bOpenedFromSaved is bad
Not to mention it's obvious. Where else would I have loaded the document from, but from a place it was previously saved? :-) > and can lead to reversed if conditions; the fixed code is: > > if (bReadSaveWriteOpen) I'd say this bool variable is not too well named. It tells me it's a boolean telling me if I should either perform, or have performed, the following sequence of operations: 1) Read, 2) Save, 3) Write and 4) Open. This obviously in this context must be a document. What document would want to be pushed through that loop? Why? It would be equally clear to me if someone added "ViewWindowFrameFileStream" to it, I'd still understand nothing from its name. > szSuffixes = IE_Exp::suffixesForFileType(m_lastSavedAsType); > else > szSuffixes = IE_Imp::suffixesForFileType(m_lastOpenedType); > > This makes it clear (at least to me) that this method is to take input > from the m_lastSavedAsType and write its output to m_lastOpenedType. Is that bool to say "Saved as a different format than originally loaded from"? Would it matter? If it matters, why? Is Save to save it using another format than responding Yes after you choose to close the document is? And on an architectural issue, why does the _document_ care about what type it was last saved as or what type is was "last" opened from (could a document be opened from anything but it's "last" type?)? Please don't flame me to death, it was a while since I looked at the AW code. :-) /Mike - please don't cc
