[GOOS] 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!

Post by 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.

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

Post by Susheel Kumar
- 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(...)
{
...
}
}
--
---
You received this message because you are subscribed to the Google Groups "Growing Object-Oriented Software" group.
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.

Post by Daniel Wellman
Hi Susheel!
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
"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
- 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(...)
{
...
}
}
--
---
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
For more options, visit https://groups.google.com/d/optout.
--
---
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
For more options, visit https://groups.google.com/d/optout.