CODEWITHSUNDEEP
separation-of-concerns
walker
walker

Reputation:

applying separation of concerns

I wonder if you think that there is a need to refactor this class.( regarding separation of concern)

publi class CSVLIstMapping<T>
{
 void ReadMappingFromAttirbutes();
 void GetDataFromList();
}

ReadMappingFromAttributes - Reads the mapping from the type T and stores it in the class. Has a name of the list to use and a number of csvMappingColumns which contains the name of the property to set the value in and the name of csvcolumns.

GetObjectsFromList - uses a CVSListreader ( which is passed in via the constructor) to get the data from all row's as KeyValuePair ( Key = csvcolumnName , value = actually value) and after that it uses the mappinginformation( listname and csvMappingColumns ) to set the data in the object.

I cant decide if this class has 2 concerns or one. First I felt that it had two and started to refactor out the conversion from rows to object to another object. But after this it felt awkward to use the functionality, as I first had to create a mappingretriver, and after that I had to retrive the rows and pass it in together with the mapping to the "mapper" to convert the objects from the rows

/w

Upvotes: 1

Views: 286

Answers (2)

annakata
annakata

Reputation: 75862

Rule of thumb: if it's awkward, it's wrong.

I have to say I'm finding it hard to understand what you've written there, but I think it's likely that you need to refactor the class: the names seem unclear, any method called GetFoo() should really not be returning void, and it may be possible that the whole ReadMappingFromAttribute should just be constructor logic.

Upvotes: 0

duffymo
duffymo

Reputation: 308988

Sounds like two concerns to me: parsing and mapping/binding. I'd separate them. CSV parsing should be a well-defined problem. And you should care about more than mere mapping. What about validation? If you parse a date string, don't you want to make sure that it's valid before you bind it to an object attribute? I think you should.

Upvotes: 1

Related Questions