Single Responsibility Class Design

[Susheel Kumar]

Hello, 

I wanted to get some thoughts/ or confirm class design for below feed scenario from Single Responsibility principle perspective and also wanted to confirm if calling DBHandler & FileHandler methods from PersonJsonParser.parse(..) method below violates SRP from any perspective and if so, how should I design.

- We have a service method to receive feeds

- The two parameters we receive are HTTP headers & String (JSON array of people/person)

- We need to parse headers, validate it 

- Parse array of people/person 

- For each person, check if it is already in DB and get their last timestamp. 

- if newer record, then write into DB along with metadata and create a file on filesystem with transformed JSON for another process to pickup

- if record exists, then compare timestamp, update it in DB and create file on filesystem with transformed JSON for another process to pickup.

- If not a latest document, then don't take any action.  

- Success / exception / failure / Any Validation / Business Rules / if document arrived was older then in database etc, need to be communicated back in response with appropriate status like (200 (OK), 400(ERROR), 500 (RETRY) 

The classes/method design I have like below. 

//FeederService

class FeederService

{

public feedPerson(String request, String headers)

{

try

{

RequestHeader reqHeader = new RequestHeader();

PersonRequestHandler personRequestHandler = new PersonRequestHandler();

reqHeader.parse(headers);

response = personRequestHandler.process(request,headers);

return response;

}catch(...)

{

}

}

//PersonRequestHandler

class PersonRequestHandler

{

public Response process(String searchRequest,RequestHeaders headers)

{

....

PersonJsonParser jsonParser = new PersonJsonParser();

try {

JSONObject mainJson = new JSONObject(searchRequest);

JSONArray personsArray = mainJson.getJSONArray("persons");

response = jsonParser.parse(personsArray,headers);

return response;

}

catch(JSONException ex) {

...

}

catch(Exception ex) {

...

}

}

}

//PersonJsonParser

class PersonJsonParser

{

...

public Response parse(JSONArray personsArray, RequestHeaders headers)  {

...

StringBuffer outDocument =  new StringBuffer();

try

{

for (int i = 0; i < personsArray.length(); i++)

{

JSONObject outputJson;

outputJson = getPersonOutputJson((JSONObject)usersArray.get(i),headers);

...

DBHandler.write(outputJson);

outDocument.append(outpuJson);

..

} // end for loop

//if some docs to be written on FileSystem

if(outDocument.length() > 1 ) {

...

FileHandler.createFile(outDocument);

...

}

} catch(Exception ex) {

...

}

//DBHandler

class DBHandler

{

...

public void write(..)

{....}

}

//FileHandler

class FileHandler

{

...

public void createFile(...)

{

...

}

}

 

[Daniel Wellman]

Hi Susheel!

My response below:

On Feb 27, 2016, at 8:38 AM, Susheel Kumar <sushe...@gmail.com> wrote:

Hello, 

I wanted to get some thoughts/ or confirm class design for below feed scenario from Single Responsibility principle perspective and also wanted to confirm if calling DBHandler & FileHandler methods from PersonJsonParser.parse(..) method below violates SRP from any perspective and if so, how should I design.

When checking the Single Responsibility Principle, I'll try the tip suggested in GOOS: try to say what the object does without using the word "and".  So I'd say:

"The job of the PersonJsonParser is to parse the Person JSON object and save the record in the database and save the appropriate file."

I don't have a full understanding of your problem yet, so I may have mischaracterized the code. You will be able to do this better than me. I did use and a few times, and the object name felt like a clue to me -- the PersonJsonParser talking to a database and a file system felt surprising; given the name I expected that it would only parse JSON.

That might mean the object is doing too many things. But it also could be that the name could be changed; it seems to me like it's performing some business process -- checking a record and notifying some dependencies about the state of that record (is it new? Did it already exist? Was it the latest version?)

Then I realized that the names seemed to focus on the technology implementation -- database, JSON, files. I wondered if you removed the technology specific names from the classes, what names would describe the process? This might lead to having your PersonJsonParser actually be a composite object that that describes the business process that tells its dependencies what happened in terms of the domain (the person was new, their information was out of date). Then the listeners could be implemented to respond with the correct technical infrastructure action (eg the updated person information is written to a file).

Perhaps your system is a mediator between two pieces if infrastructure; in that case the technical names may be the best way to describe the domain. If that's the case I would try to look for some way to model the infrastructure transformation process and see if changing the names revealed something about the responsibilities.

Cheers

Dan

--

---
You received this message because you are subscribed to the Google Groups "Growing Object-Oriented Software" group.
To unsubscribe from this group and stop receiving emails from it, send an email to growing-object-oriente...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Susheel Kumar]

Thanks, Daniel for detailed response.  Some of the code i inherited/legacy which i am trying to refactor and part of that coming up with right names for objects/processes. I'll share the newer objects/details to clarify.