43
edits
Difference between revisions of "User talk:David corrales"
Jump to navigation
Jump to search
Added the new Common::File redesign brief
(→Misc) |
(Added the new Common::File redesign brief) |
||
| Line 190: | Line 190: | ||
|} | |} | ||
== Common::File redesign discussion == | == Common::File redesign discussion (old)== | ||
While analizing the current state of the Common::File class, I've been noticing several important things that I feel, should be discussed. | While analizing the current state of the Common::File class, I've been noticing several important things that I feel, should be discussed. | ||
First of all, is the relevance of breaking up this class (like the FilesystemNode) into different implementations for each architecture. This is the primary motivation for this section. | First of all, is the relevance of breaking up this class (like the FilesystemNode) into different implementations for each architecture. This is the primary motivation for this section. | ||
| Line 282: | Line 282: | ||
=== New methods === | === New methods === | ||
Adding new methods to the existing Common::File class wouldn't have an impact, unless it redefines or changes the signature of the old ones. Instead of breaking up said class, it could be cleaned up and extended with new functionality. | Adding new methods to the existing Common::File class wouldn't have an impact, unless it redefines or changes the signature of the old ones. Instead of breaking up said class, it could be cleaned up and extended with new functionality. | ||
== New Common::File redesign proposal and implementation == | |||
After being involved in the project for more time, I analyzed the current design and created the following proposal to address the Common::File class. | |||
Reading through the cpp file, I discovered that most discrepancies occur in the redefinition of standard I/O manipulation functions (e.g. fopen, ftell, fseek and so on) in 3 platforms: ds, ps2 and symbian. There's some #ifdef's around, but the algorithms are 99% the same for all platforms, so a separate implementation for each architecture is not needed and instead, would result in unnecessary code duplication. | |||
Given this situation the template pattern looked like a really good alternative: create a base file class that implements the algorithm but allows subclasses to choose which method they want for fopen, fclose, etc calls. This is done through protected functions named _fopen, _close, etc. | |||
Additionally, the current Common::File class is mixing responsibilities that go beyond a regular file class. For example, the whole default directory structure (contained in a hash map for quick access) is beyond the scope of a regular file manipulation object. The objective is then to convert Common::File into a wrapper for BaseFile, just like Common:FsNode is around AbstractFilesystemNode. | |||
The responsibilities per function would be: | |||
{| border="1" cellpadding="5" cellspacing="0" | |||
!Function | |||
!BaseFile | |||
!Common::File | |||
|- | |||
|void clearIOFailed | |||
|Clear the last I/O error flag | |||
|Checks + Call the base class | |||
|- | |||
|virtual void close | |||
|Close the open file | |||
|Checks + Call the base class | |||
|- | |||
|bool eof | |||
|Whether the seek pointer is at the EOF | |||
|Checks + Call the base class | |||
|- | |||
|bool eos | |||
|Whether the seek pointer is at the EOS | |||
|Checks + Call the base class | |||
|- | |||
|bool ioFailed | |||
|Whether the last I/O failed | |||
|Checks + Call the base class | |||
|- | |||
|const char *name | |||
|Name of the stream | |||
|Checks + Call the base class | |||
|- | |||
|bool isOpen | |||
|Whether the file is open | |||
|Checks + Call the base class | |||
|- | |||
|virtual bool open | |||
|Open a given filepath | |||
|Use the default paths to locate the filepath, then call the base class | |||
|- | |||
|uint32 pos | |||
|Position of the seek pointer within the file | |||
|Checks + Call the base class | |||
|- | |||
|uint32 read | |||
|Reads a chunk from a file | |||
|Checks + Call the base class | |||
|- | |||
|virtual bool remove | |||
|Remove a file | |||
|Checks + Call the base class | |||
|- | |||
|void seek | |||
|Move the seek pointer | |||
|Checks + Call the base class | |||
|- | |||
|uint32 size | |||
|Size of the file | |||
|Checks + Call the base class | |||
|- | |||
|void addDefaultDirectory | |||
| - | |||
|Add a default search path | |||
|- | |||
|void addDefaultDirectoryRecursive | |||
| - | |||
|Recursive method of addDefaultDirectory | |||
|- | |||
|void resetDefaultDirectories | |||
| - | |||
|Clear default directories | |||
|- | |||
|bool exists | |||
| - | |||
|Whether the file exists. Uses Common:FSNode | |||
|- | |||
|} | |||
The wrapper will handle the whole default search path functionality plus checks and maybe some caching for the BaseFile object, again, just like Common::FsNode does. | |||
I did not list the write functions here, since a while ago I recall a discussion that implied removing the write abilities from the Common::File class. This of course needs to be defined clearly. | |||
Another issue to be resolved is BaseScummFile inheriting Common::File. Max told me it was possible to code BaseScummFile as a wrapper instead of an inherited class, so that could be a possible solution. | |||
The following diagram shows the class structure: | |||
== Current Filesystem API implementation diagrams== | == Current Filesystem API implementation diagrams== | ||