trjhtr

My program is iterating over list of files and processing each of them using some library. It's possible that file might be corrupted in the process so I want to backup it to prevent potential corruption.

I implemented it this way:

private static void ComprocessFiles(List<string> files)

 FileInfo file = null; 
 foreach (string filepath in files)
 
 bool backedUp = false;
 bool keepBackup = false;
 try
 
 file = new FileInfo(filepath);
 if (!file.Exists) continue;
 if (file.IsReadOnly) continue;

 BackupHelper.Backup(file);
 backedUp = true;
 ProcessFile(file);

 BackupHelper.RestoreTimestamps(file);
 file.Refresh();
 
 catch (Exception ex)
 
 finally
 
 if (!IsCorrupted(file)) keepBackup = true;
 if (!keepBackup) BackupHelper.DeleteBackup(file);
 
 

BackupHelper class:

static class BackupHelper

 static private DateTime timestamps = new DateTime[3];

 static internal void Backup(FileInfo originalFile)
 
 timestamps[0] = originalFile.CreationTime;
 timestamps[1] = originalFile.LastWriteTime;
 timestamps[2] = originalFile.LastAccessTime;

 File.Copy(originalFile.FullName, originalFile.FullName + "_backup", true);
 

 static internal void Backup(string originalFile)
 
 Backup(new FileInfo(originalFile));
 

 static internal void RestoreFile(FileInfo originalFile)
 
 File.Copy(originalFile.FullName + "_backup", originalFile.FullName, true);
 

 static internal void RestoreFile(string originalFile)
 
 RestoreFile(new FileInfo(originalFile));
 

 static internal void RestoreTimestamps(FileInfo originalFile)
 
 originalFile.CreationTime = timestamps[0];
 originalFile.LastWriteTime = timestamps[1];
 originalFile.LastAccessTime = timestamps[2];
 

 static internal void RestoreTimestamps(string originalFile)
 
 RestoreTimestamps(new FileInfo(originalFile));
 

 static internal void DeleteBackup(FileInfo originalFile)
 
 timestamps = new DateTime[3];
 File.Delete(originalFile + "_backup");
 

 static internal void DeleteBackup(string originalFile)
 
 DeleteBackup(new FileInfo(originalFile));
 

I tried my best as I plan to publish source code online but still feel that there is room for improvement.

I guess methods ProcessFile() and IsCorrupted() are not important for that question as well as Logger class.

Important note - StackOverflowException and OutOfMemoryException can't be thrown by this code as it don't do many method calls and don't use much memory.

How great is the risk that your files will be corrupted? Is there a particular reason why you think this might happen, and if so, is that cause something you can address?

Are you concerned that it is your act of processing the file that might corrupt it? If so, you may want to do the opposite of what you're doing. Don't create a backup, but instead leave the original alone and create a working copy which you discard when you're done. If something goes wrong then at least your original file is still there, untouched.

This may sound facetious but it's not: If you don't trust your file system then it's not a good place for your application's data. How do you know that even after backing up, the file you're reading won't become corrupt as your're reading it? How do you know it wasn't already corrupt when it was backed up? How do you know that the backups won't be corrupted?

The best approach depends not only on how much you trust (or distrust) your storage but also how great the risk is if your data gets lost. If your server gets backed up regularly then that might be all the protection you need. Perhaps you could create a hash or a checksum for each file and store it with the file. When you process the file, validate that it matches the hash. That gives you about as much certainty as you could possibly want.

I am a little concerned about the static fields used to store timestamps in your BackupHelper class. If someone was using your class in different threads, or backing up more than one at a time, processing them all, then restoring them all, they would potentially end up with incorrect timestamp fields.

One way to resolve this might be to use a Dictionary<string, List<string>>, where the key is the filePath and the timestamps are the value.

Another thing would be to store the string "_backup" as a const. Repeated hard-coded strings can sometimes lead to problems.

All I have for now

I don't like

File.Delete(originalFile + "_backup");

You are passing a FileInfo. I have not tested but I suspect you are getting an implicit cast to string.

FileInfo has a Delete method. Have BackupHelper return a FileInfo so you can delete directly.