# Synchronizer for importing XML files into a database when folder content changes

I have created a Synchronizer, the purpose of which is to read data from an XML source file and store the result in a DB.

I have different source types, for example Student.XML, School.XML, etc. These files are copied in my source directory, C:\Source.

My Synchronizer watches the Source folder, and every time there is a new file, it should read it, map it to my domain class and write the domain class into the DB.

If C:\Source folder contains Student.XML, it should read the content of the XML file and copy it in Student Table in the DB. If C:\Source contains School.XML, it should read the file and copy the content to School Table.

To solve this, I have defined an ISyncer interface:

public interface ISyncer
{
// Read input and synchronize destination, return error message if any
string Sync();
}


Now all my Syncer types (e.g. StudentSyncer, SchoolSyncer, etc) should implement this interface:

public class StudentSyncer : ISyncer
{
// simple XML reader, reads XML file into a list of List<XMLStudent>

public StudentSyncer(string file)
{
}

public string Sync()
{
// read the input

// using automapper map to domain object
List<Student> dbStudent = Mapper.Map<List<XMLStudent>, List<Student>>(source);
dbStudent.ForEach(s => s.IsCurrent = true);                // set IsCurrent

// write records to Student table, using Unit of Work patterns
using (var context = new DbContext())
{
UnitOfWork uow = new UnitOfWork(context);

// set the existing records to not current
var curSet = uow.Student.FindByTrackingChanges(s => s.IsCurrent == true).ToList();
curSet.ForEach(s => s.IsCurrent = false);
uow.SaveChanges();
}

return "";  // no error
}
}


I have a Windows service, which calls the DoSync() method every 30 seconds:

public void DoSync()
{
foreach (string file in Directory.EnumerateFiles(@"C:\Source", "*.xml"))
{
// use syncer factory to initialize the correct instance of syncer based on input file name
ISyncer syncer = _syncerFactory.CreateInstance(file);

// do the synchronization task
syncer.Sync();

// Move file to processed folder
MoveFile(@"C:\Destination");
}
}


The Factory is a simple factory. It reads the source file name, e.g. Student.XML, School.XML and based on the file name. It initializes the correct Syncer, e.g. StudentSyncer, SchoolSyncer.

I am happy with the code, but it is bothering me because I feel it is not following the Single Responsibility Principle. My Syncer class is not doing 1 task, but it is doing 3 tasks: read input, map to domain class, write to DB.

• Synchronizer should watch the Source folder - where is the code that does that? Have you removed anything? – t3chb0t Jun 22 '18 at 9:42
• @t3chb0t, thanks. It's a Windows event handler which calls DoSync () every 30 seconds. I did not include that part of the code in the review as I thought it's straight forward ... can add it, if it is necessary. – Hooman Bahreini Jun 22 '18 at 9:50

I know this is a little late, but instead of the timed call to DoSync(), you could consider to use FileSystemWatcher and then subscribe to the relevant events.

• Thanks for your answer. Yes, that's an alternative and @t3chb0t gave the same comment... my main question is about Single Responsibility Principle... at what point do we say that a class has too many responsibilities and we should refactor it? The best article that I have seen is this one by Nikola Malovic... I think Nikola's 2nd law of Ioc answers my question above: "Any class having more then 3 dependencies should be questioned for SRP violation" – Hooman Bahreini Mar 22 '19 at 10:51
• @HoomanBahreini: OK, I haven't seen that comment. About SRP: This can be obtained on all levels of a project (modules, classes, methods) and depends on the abstraction level IMO: On one level of abstraction your Syncer does only one thing (syncronize a db from a xml file) but on another it does three: read, map, save - and one an even lower level the reading is about reading each byte from a stream etc.... In each project you have to find a reasonable ratio between SRP and fragmentation. I agree that you question may be more about IoC than SRP. – user73941 Mar 22 '19 at 11:15

How about extracting tasks common to all source types into an (abstract) base class that, in turn, implements your ISyncer interface?

I would also change the error reporting by using exceptions, rather than have the method return a string in case of error.

Edit, code example (C#-ish pseudo code)

abstract class SyncerBase : ISyncer
{
sealed List<T> Map(List source, List target)
{
// Mapping magic here
}

sealed SaveToDB(List dataToSave, params ...)
{
// Your database access here
}
}


Put utility methods and common stuff for all your synchronizers in that base class, and inherit it.

class StundentSyncer : SyncerBase
{
// Constructor as in your code
void Sync(string file)
{