The refactoring consists of:
- Changing writeOut & writeOutForFile API
to return boolean instead of MutableDomItem, which better reflects
the existing usecases improving consistency of the data model*
Moreover, previous API was exposing DomItem, which was not "committed
to base" (MutableDomItem.commitToBase()), meaning it was exposing the
"unmerged" Item alongside with the "temporary environment"
- Refactoring & renaming OutWriter::updatedFile
breaking it into smaller chunks preserving
only necessary functionality
- Adding some comments / documentation
Before this commit, the writeOut API was "exposing",so called,
"updatedFile", which is basically the copy of the original fileItem +
renewed scriptExpressions which were modified during the writeOut of
the original fileItem.
The idea behind the "mutating" Dom API is that one has to create a
MutableDomItem, do some changes to it and then "commit" them.
This process is also facilitated by the creation of separate Env.
(git analogy might be handy here:
We create a separate branch, where all the mutation will happen and then
we "merge" this branch)
However, in the writeOutForFile usecase this "updatedFile" was needed
only for the verifying of the consistency of the "writtenOut" DOM,
however the API was exposing it further back to the caller sites,
without "committing".
The potential issue here is inconsistency of the data Model.
On one side we have an original File Item owned by the Base Env,
on the other side we have an "updatedFile" which is owned by another Env.
Taking into account that there are no usecases requiring "exposing"
"updatedFile", but also no need for "committing" the changes,
It's arguably better to keep that temporary "updatedFile" locally,
not exposing it outside the writeOutForFile function. Thereby improving
consistency of the data model.
Change-Id: If45eca4b4d6d703e2a76d0580f124d0292af5ed8
Reviewed-by: Semih Yavuz <semih.yavuz@qt.io>