HiveBrain v1.2.0
Get Started
← Back to all entries
snippetcsharpMinor

Implemenation for concurrent file access (read/write)

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
filereadwriteimplemenationforconcurrentaccess

Problem

I am developing an application that works with files and folder on a network share (similar to the windows explorer). The application is used by multiple users and provides some commands for modifying the file system.

Problem

The parallel access to the file system may result in conflicts.

Example:

User A renames a folder (FolderA) while user B is executing a long-running process that moves / creates / deletes some of the sub folders of folder B. Therefore the rename breaks the operation of user B.

Requirements

  • An application should be able to lock folders if required.



  • Other applications should be notified as soon as possible about locks, so that these applications may disable corresponding commands (e.g. Rename will be disabled for a folder where any child folder is locked).



  • All kinds of network shares should be supported (windwos, samba, ...)



  • The application should be able to show the locking user / and timestamp.



Approach

To avoid conflicts as described above, a centralized locking mechanism should be introduced.

The idea is, to create a file on the network share. That file contains all current locks (one per line). Format: "RelativePath|User|DateTime".

Each application checks in certain intervals (e.g. 100 ms) if the file changed and updates it's internal state accordingly.

Each application may add / remove entries to / from the file.

Implementation

The API to the storage uses the following simple interface:

ILockStorage.cs

public interface ILockStorage : IDisposable
{
    event EventHandler LockEntriesChanged;

    void AddEntry(LockEntry entryToAdd);
    void RemoveEntry(LockEntry entryToRemove);
}


LockEntriesChangedEventArgs.cs

```
public class LockEntriesChangedEventArgs : EventArgs
{
private readonly LockEntry[] myEntriesAdded;
private readonly LockEntry[] myEntriesRemoved;

public LockEntriesChangedEventArgs(LockEntry[] entriesAdded, LockEntry[] EntriesRemoved)
{
myEntriesAdded = entriesAdded;

Solution


  • There is no need to cast ((IEquatable)this).Equals(other);, you can just call Equals(other)



-
It's weird to use both Equals and == for the same type in the same bool expression:

myUser == other.User &&
myPath.Equals(other.Path);


Just stick to one or the other, so reader does not have to look for deeper meaning to all this. :)

  • SHOUT_CASE is unusual in C#. You should use PascalCase for constants instead: WriteRetryCount.



  • I think you should extract constants to separate class, so you can easily serialize it later on and tweak those values without re-compilation. You can call it LockStorageSettings or something. You can move filePath and timer interval there as well.



  • You shouldn't use nested classes to build class hierarchy. I'd say, that if nested class is larger than 50 lines - it probably deserves to be moved to dedicated .cs file.



  • Default encoding may vary depending on your users' environments. Since you are using a shared file, you have to specify encoding explicitly when writing and reading, to make sure that all the users are using the same encoding.



  • OpenWriteStream does too much: it opens the stream AND it parses the entries. Split it into two methods and replace out with proper return value. Nobody likes using out. :)



  • I think Write method is the wrong method to override. I mean writing logic does not change, its fixed: you take a bunch of entries, and you write them one by one. What changes is which entries you have to write. So instead of abstract Write you should have abstract IEnumerable GetUpdatedEntries(IEnumerable oldEntries).



  • Your LockStorage class is tightly coupled with LockEntryWriter class which is a sign of bad design. LockEntryWriter should have a single responsibility: writing entries to file. It should not manage timer or update cache of parent component. IMHO, you should remove LockStorage reference from LockEntryWriter constructor.

Code Snippets

myUser == other.User &&
myPath.Equals(other.Path);

Context

StackExchange Code Review Q#133763, answer score: 7

Revisions (0)

No revisions yet.