Re: commit: 2746

From: Patrick Lam (plam@plam.lcs.mit.edu)
Date: Thu Mar 21 2002 - 16:35:04 EST

  • Next message: Dom Lachowicz: "Commit: more mem leaks fixed"

    On Thu, Mar 21, 2002 at 10:27:31PM +0100, Mike Nordell wrote:
    > 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? :-)

    The method is actually supposed to do the following thing.

    The document keeps track of the last saved and last opened filetype.
    When you open a file, you want to synchronize the last opened and last
    saved filetypes.

    > > 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.

    I think that the whole method is badly designed. But I'm not out to fix
    that at the moment. The method really does two different things, which
    happen to share most of the same code.

    > 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?)?

    The Save As dialog needs to have a filetype to save as.

    pat



    This archive was generated by hypermail 2.1.4 : Thu Mar 21 2002 - 16:35:20 EST