Refactorings, code improvements and bug fixes #36
Conversation
-Resharper Code Cleanup -Removed redundant code -Fixed issues in stream structure description(comment) -Removed usage of ArrayList in favor of List<T> - yes it was used xD -Sorted some of the properties/methods in files -Improved readibility -Cleanup of usages -Fixed few bugs on the way -Improved execution time slighty -"Fixed" equals to do the same as other equal - yet it seems wrong. #Massive changes in general, get it for free or leave it. I do hope it will help you. It still works as before.
|
Massive thanks @DevDrake , it will take some time to check all your work before merging, but meanwhile please let me kindly know what you have identified as "bug" (>>Fixed few bugs on the way) because I should give high priority to those changes that fixes functional errors. |
| /// </example> | ||
| public abstract class CFItem : IComparable<CFItem> | ||
| public abstract class CFItem : IComparable<CFItem>, IEquatable<CFItem> |
There was a problem hiding this comment.
Added The IEquatable interface to complete the IComparable
| } | ||
|
|
||
| // If one is null, but not both, return false. | ||
| if (((object)leftItem == null) || ((object)rightItem == null)) |
There was a problem hiding this comment.
Fixed this, the comment sugest different behavior than written. Implemented the intended XOR
| /// Checks the underlying stream without throwing exception. | ||
| /// </summary> | ||
| /// <returns>true if stream is closed, false if stream is open.</returns> | ||
| protected bool IsDisposed() |
There was a problem hiding this comment.
Added the no throwing version of checking Disposed status for Try_ functions
|
|
||
| #region IComparable Members | ||
|
|
||
| public int CompareTo(CFItem other) |
There was a problem hiding this comment.
Fixed the implementation of IComparable for the class
| get { return miniSectorShift; } | ||
| } | ||
| // 34 | 6 | Not used | ||
| // 40 | 4 | Total number of sectors used Directory (➜5.2) |
| internal int CompareTo(CFItem other) | ||
| { | ||
|
|
||
| return this.dirEntry.CompareTo(other.DirEntry); |
There was a problem hiding this comment.
In General, this seems to be incorrect as we omit the second property CompoundFile - but i am not sure if the current implementation relies on that in any way, therefore i left it hanging like that.
| /// </example> | ||
| public bool TryGetStorage(String storageName, CFStorage cfStorage) | ||
| public bool TryGetStorage(string storageName, out CFStorage cfStorage) |
There was a problem hiding this comment.
Fixed the API to actually return the value
|
Hi @ironfede, first, thanks for sharing the OpenMCDF with the community! |
-Resharper Code Cleanup
-Removed redundant code
-Fixed issues in stream structure description(comment)
-Removed usage of ArrayList in favor of List - yes it was used xD
-Sorted some of the properties/methods in files
-Improved readibility
-Cleanup of usages
-Fixed few bugs on the way
-Improved execution time slighty
-"Fixed" equals to do the same as other equal - yet it seems wrong.
#Massive changes in general, get it for free or leave it. Leaving it as a token of gratitude.
It still works as before.