I’m trying to design a generic interface for enumerating over a list files. Each iteration will open a file, allow the consuming code to access the file to perform some task, and then close the file.
At first this seemed like like a simple enumeration pattern, but the consuming code actually needs to know the status of each file both after it is opened and after it is closed. So rather then having a single MoveNext() method my interface has OpenNextFile() and CloseCurrentFile() methods, each of which may return a failure status. In addition, CloseCurrentFile() needs a way to report whether each file was saved or not when it was closed.
Here’s my current interface.
public interface IFileEnumerator
{
//The following properties reflect the current state of the IFileEnumerator
FileEnumeratorStatus Status { get; }
string StatusMessage { get; }
string FileName { get; }
byte[] FileData { get; }
//Attempts to open the next file
FileEnumeratorStatus OpenNextFile();
//Closes the current file
FileEnumeratorStatus CloseCurrentFile();
}
public enum FileEnumeratorStatus
{
AtStart, //Files have not been opened
Opened, //File currently opened
Opened_Failed, //Failed to open file
Close_Saved, //File closed and saved
Closed_NotSaved, //File closed and not saved
Close_SaveFailed, //File closed saving failed
AtEnd //No more files
}
Here’s an example of the consuming code:
//FileProcessor is a class which run a ProcesssJob on a set of files returned by a
//IFileEnumerator and returns a ProcesssingJobResult with results of all the files
public class FileProcessor
{
public ProcesssingJobResult RunJob(IFileEnumerator fileEnumerator, ProcesssJob job)
{
List<FileProcessingResult> resultList = new List<FileProcessingResult>();
while (fileEnumerator.OpenNextFile() != FileEnumeratorStatus.AtEnd)
{
FileProcessingResult currentFileResult = new FileProcessingResult();
currentFileResult.FileName = fileEnumerator.FileName;
if (fileEnumerator.Status == FileEnumeratorStatus.Opened)
{
currentFileResult.JobCompleted = job.RunJob(fileEnumerator.FileData);
var closureStatus = fileEnumerator.CloseCurrentFile();
if (closureStatus == FileEnumeratorStatus.Close_Saved)
currentFileResult.Saved = true;
else if (closureStatus == FileEnumeratorStatus.Closed_NotSaved)
currentFileResult.Saved = false;
else if (closureStatus == FileEnumeratorStatus.Close_SaveFailed)
{
currentFileResult.Saved = false;
currentFileResult.Message = fileEnumerator.StatusMessage;
}
}
else if (fileEnumerator.Status == FileEnumeratorStatus.Opened_Failed)
{
currentFileResult.JobCompleted = false;
currentFileResult.Message = fileEnumerator.StatusMessage;
currentFileResult.Saved = false;
}
resultList.Add(currentFileResult);
}
ProcesssingJobResult jobresult = new ProcesssingJobResult();
jobresult.TimeStamp = DateTime.Now.ToUniversalTime();
jobresult.Machine = System.Environment.MachineName;
jobresult.User = System.Environment.UserName;
jobresult.Results = resultList;
return jobresult;
}
}
//A generic task that can be run on a file
public interface ProcesssJob
{
bool RunJob(byte[] data);
}
//Result of processing one file
public class FileProcessingResult
{
public bool JobCompleted { get; set; }
public bool Saved { get; set; }
public string FileName { get; set; }
public string Message { get; set; }
}
//The overall result of processing a set of files
public class ProcesssingJobResult
{
public DateTime TimeStamp { get; set; }
public String Machine { get; set; }
public String User { get; set; }
public List<FileProcessingResult> Results { get; set; }
}
Maybe this is just a question of careful documentation, but I’m having trouble dealing with all these different possible states. Drawing a state diagram doesn’t help as it’s possible to go from every state to just about every other state. Should the consuming code call CloseCurrentFile() if OpenNextFile() return Open_Failed? What if it does? Would a Closed status be returned even if a file was never opened? If CloseCurrentFile() was not called after OpenNextFile() should it be closed automatically by the next call to OpenNextFile() or should an exception be thrown? I’d rather someone implementing this interface not have to worry about getting all this right.
The problem is much simpler if I have a single OpenNextFile() method which is responsible for both closing the last opened file, and opening the next file. Yet then this single method needs to report both the result of opening the next file, as well as closing the last file. This also becomes tricky for consuming code as the final state of each file is not determined until the following iteration.
public interface IFileEnumerator_Alternate
{
string FileName { get; }
byte[] FileData { get; }
string StatusMessage {get;}
//Closes the previous file, and opens the next file
Result OpenNextFile();
}
public class Result
{
public bool CurrentFileOpened {get; set;}
public string CurrentFileName {get; set;}
public string CurrentFileStatusMessage {get; set;}
public string LastFileName {get; set;}
public bool LastFileClosedOk {get; set;}
public string LastFileClosedStatusMessage {get; set;}
public bool LastFileSaved {get; set;}
}
Is there another way to look at this, or some other common design pattern that is more suited to what I’m trying to do? Here’s an activity diagram showing the overall process.
11
What about approaching the problem from the other side. You have a class, that represents that nice flow diagram of yours and this flow diagram creates “events” based on transitions. Those events then can be represented as either an interface or events:
public interface IFileProcessor
{
void HandleFile(string fileName, Stream data);
void OpenFailed(string fileName);
void CloseFailed(string fileName);
void Closed(string fileName, bool saved);
void AllFilesProcessed(); // maybe?
}
Then your “enumerator” will accept instance of class that implements this interface and executes the relevant methods.
string[] files = ...; // what files?
IFileProcessor processor = ...; // how the files should be handled?
var enumerator = new FileEnumerator(files, processor);
enumerator.Run(); // process all files
This solutions seems much simpler, but slightly harder to understand due to inverted control.
12
What if instead of using an interface I create an abstract class with abstract methods of each of these steps which concrete classes have to implement
Yes. An interface does not tell how the methods interact or how to process. That is just one technique for dependency injection.
Part of the problem is acquiring the list of files can be a long process. Each file may need to be downloaded, and opened to find nested contents etc..etc. So I want to do the processing as the files are being iterated over rather then opening them twice.
Document Facade
A general meta-data class & and associated “framework” with injectable custom equality and comparison functionality.
public class DocumentFacade : IEquatable<DocumentFacade>, IComparable<DocumentFacade> {
// this class is more like document metadata, not necessarily a facade design
// pattern wrapping the Document object itself
protected EqualityComparer<DocFacade> equalizer; // customized Equals, CompareTo
public string Name { get; set; }
// superset of any and all properties relevant to processing. Covers all
// document sub classes.
public DocStatus OpenStatus { get; set; } // an enum
public DocStatus CloseStatus { get; set; }
public DocClass DocumentClass; // an enum, alternatively Type.Name, or even the Type object itself.
public BaseDocClass theDoc; //reference to the document this DocumentFacade represents
// maybe you don't even need this.
public DocumentFacade (IEqualityComparer<DocFacade> equalizer) {
if(equalizer == null) throw new NullArgumentException();
this.equalizer = equalizer;
}
public override bool Equals(object that) {
if(that == null) return false;
if(that.DocumentType != this.DocumentType) return false;
return equalizer.Equals(this, other);
}
// the IEquatable implementation
public bool Equals(this, y) {
return Equals(y);
}
}
// A class for each way we want to compare Documents; each sub-class I suppose.
// MSDN says inheriting is the way to go, vice interface implementation.
public class DocFacadeEqualizerTypeA : IEqualityComparer<DocumentFacade> {
public override bool Equals (DocFacade x, DocFacade y) {
// customized implementation
}
// ditto CompareTo override
}
public class DocumentFacadeCollection : List<DocFacade> {
public bool Equals (DocFacade x, DocFacade y) {
if(x == null) return false;
return x.Equals(y);
}
// lots of inherited List stuff makes this class really handy.
public DocFacadeCollection GetOpenStatus(DocStatus thisStatus) {
Find( x => x.OpenStatus == thisStatus);
}
public DocFacadeCollection FindAllDuplicates (DocFacadeCollection otherList) {
DocFacadeCollection duplicates = new DocFacadeCollection();
duplicates.AddRange( this.FindAll( x => otherList.Contains( x )).AsEnumerable());
return duplicates;
}
}
public static class DocumentDNAAnalyzer {
// this may be a factory, or perhaps more likely, the front of
// of a Builder pattern implimentation
// Besides a customized DocFacade object, we may also be able to
// provide stuff for abstract template.
public static DocFacadeFactory = new DocFacadeFactory();
public static DocFacade Create(object TheDocument) {
return DocFacadeFactory.Create(TheDocument));
}
}
See this thread for a great GetHashCode() Implementation
I think that you should separate enumerating over files and handing each particular file.
There already is an interface in .Net for enumerating: IEnumerable<T>, so I would use that.
That leaves working with a single file: that would look pretty much like your IFileEnumerator, except with the enumerator parts removed.
I also don’t think that opening and closing should use the same status enum.
public interface IFile
{
string FileName { get; }
byte[] FileData { get; }
bool Open();
FileCloseStatus Close();
}
public enum FileCloseStatus
{
Saved, //File closed and saved
NotSaved, //File closed and not saved
SaveFailed, //File closed saving failed
}