Synchronizer for importing XML files into a database when folder content changes
Clash Royale CLAN TAG#URR8PPP
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;
up vote
3
down vote
favorite
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.
c# object-oriented database xml factory-method
add a comment |Â
up vote
3
down vote
favorite
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.
c# object-oriented database xml factory-method
Synchronizer should watch the Source folder - where is the code that does that? Have you removed anything?
â t3chb0t
Jun 22 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
Jun 22 at 9:50
add a comment |Â
up vote
3
down vote
favorite
up vote
3
down vote
favorite
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.
c# object-oriented database xml factory-method
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.
c# object-oriented database xml factory-method
edited Jul 5 at 2:31
asked Jun 22 at 3:50
Hooman
1387
1387
Synchronizer should watch the Source folder - where is the code that does that? Have you removed anything?
â t3chb0t
Jun 22 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
Jun 22 at 9:50
add a comment |Â
Synchronizer should watch the Source folder - where is the code that does that? Have you removed anything?
â t3chb0t
Jun 22 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
Jun 22 at 9:50
Synchronizer should watch the Source folder - where is the code that does that? Have you removed anything?
â t3chb0t
Jun 22 at 9:42
Synchronizer should watch the Source folder - where is the code that does that? Have you removed anything?
â t3chb0t
Jun 22 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
Jun 22 at 9:50
@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
Jun 22 at 9:50
add a comment |Â
1 Answer
1
active
oldest
votes
up vote
1
down vote
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
thanks. Exception is very good advice. I am not sure what you mean about abstracting the common functinality of all types?
â Hooman
Jun 22 at 10:11
@Hooman you mention that you have StudentSyncer, SchoolSyncer, etc - it's very likely that they repeat the same code but it's hard to say because we don't see the other implemenations.
â t3chb0t
Jun 22 at 10:17
What is type T? And how to initialize Syncer / SyncerBase using a factory pattern? The reason I did not include Reader, Writer and Mapper inside ISyncer, was that I wanted to easily initialize the Syncers in a Factory.
â Hooman
Jun 23 at 0:44
add a comment |Â
1 Answer
1
active
oldest
votes
1 Answer
1
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
1
down vote
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
thanks. Exception is very good advice. I am not sure what you mean about abstracting the common functinality of all types?
â Hooman
Jun 22 at 10:11
@Hooman you mention that you have StudentSyncer, SchoolSyncer, etc - it's very likely that they repeat the same code but it's hard to say because we don't see the other implemenations.
â t3chb0t
Jun 22 at 10:17
What is type T? And how to initialize Syncer / SyncerBase using a factory pattern? The reason I did not include Reader, Writer and Mapper inside ISyncer, was that I wanted to easily initialize the Syncers in a Factory.
â Hooman
Jun 23 at 0:44
add a comment |Â
up vote
1
down vote
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
thanks. Exception is very good advice. I am not sure what you mean about abstracting the common functinality of all types?
â Hooman
Jun 22 at 10:11
@Hooman you mention that you have StudentSyncer, SchoolSyncer, etc - it's very likely that they repeat the same code but it's hard to say because we don't see the other implemenations.
â t3chb0t
Jun 22 at 10:17
What is type T? And how to initialize Syncer / SyncerBase using a factory pattern? The reason I did not include Reader, Writer and Mapper inside ISyncer, was that I wanted to easily initialize the Syncers in a Factory.
â Hooman
Jun 23 at 0:44
add a comment |Â
up vote
1
down vote
up vote
1
down vote
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
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
edited Jun 22 at 10:56
answered Jun 22 at 9:37
TomG
34115
34115
thanks. Exception is very good advice. I am not sure what you mean about abstracting the common functinality of all types?
â Hooman
Jun 22 at 10:11
@Hooman you mention that you have StudentSyncer, SchoolSyncer, etc - it's very likely that they repeat the same code but it's hard to say because we don't see the other implemenations.
â t3chb0t
Jun 22 at 10:17
What is type T? And how to initialize Syncer / SyncerBase using a factory pattern? The reason I did not include Reader, Writer and Mapper inside ISyncer, was that I wanted to easily initialize the Syncers in a Factory.
â Hooman
Jun 23 at 0:44
add a comment |Â
thanks. Exception is very good advice. I am not sure what you mean about abstracting the common functinality of all types?
â Hooman
Jun 22 at 10:11
@Hooman you mention that you have StudentSyncer, SchoolSyncer, etc - it's very likely that they repeat the same code but it's hard to say because we don't see the other implemenations.
â t3chb0t
Jun 22 at 10:17
What is type T? And how to initialize Syncer / SyncerBase using a factory pattern? The reason I did not include Reader, Writer and Mapper inside ISyncer, was that I wanted to easily initialize the Syncers in a Factory.
â Hooman
Jun 23 at 0:44
thanks. Exception is very good advice. I am not sure what you mean about abstracting the common functinality of all types?
â Hooman
Jun 22 at 10:11
thanks. Exception is very good advice. I am not sure what you mean about abstracting the common functinality of all types?
â Hooman
Jun 22 at 10:11
@Hooman you mention that you have StudentSyncer, SchoolSyncer, etc - it's very likely that they repeat the same code but it's hard to say because we don't see the other implemenations.
â t3chb0t
Jun 22 at 10:17
@Hooman you mention that you have StudentSyncer, SchoolSyncer, etc - it's very likely that they repeat the same code but it's hard to say because we don't see the other implemenations.
â t3chb0t
Jun 22 at 10:17
What is type T? And how to initialize Syncer / SyncerBase using a factory pattern? The reason I did not include Reader, Writer and Mapper inside ISyncer, was that I wanted to easily initialize the Syncers in a Factory.
â Hooman
Jun 23 at 0:44
What is type T? And how to initialize Syncer / SyncerBase using a factory pattern? The reason I did not include Reader, Writer and Mapper inside ISyncer, was that I wanted to easily initialize the Syncers in a Factory.
â Hooman
Jun 23 at 0:44
add a comment |Â
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f197024%2fsynchronizer-for-importing-xml-files-into-a-database-when-folder-content-changes%23new-answer', 'question_page');
);
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Synchronizer should watch the Source folder - where is the code that does that? Have you removed anything?
â t3chb0t
Jun 22 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
Jun 22 at 9:50