Nsryjdtyk

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>

    private XMLStudentReader _reader;



    public StudentSyncer(string file)

    {

        _reader = new XMLStudentReader(file);

    }



    public string Sync()

    {

        // read the input

        List<XMLStudent> source = _reader.ReadAll();



        // 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.Student.AddRange(dbStudent);

            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 (S of SOLID). My Syncer class is not doing 1 task, but it is doing 3 tasks: read input, map to domain class, write to DB.

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)

    {

        var XMLstudents = _reader.ReadAll();

        var students = Map(XMLstudents); // calling base class here!

        // Manipulate student list here as needed

       SaveToDB (students, database params here) // base class here, too

    }

}