I have WCF method that looks something like this:
public GetRecordsResponse GetRecords(GetRecordsRequest request)
{
GetRecordsResponse response = new GetRecordsResponse();
Util.GetRecords.VerifyMandatoryParameters(request);
//make sure mandatory parameters have correct values.
Util.GetRecords.SetDefaultValues(request);
开发者_运维问答 Util.GetRecords.SetDefaultResponseValues(request, response);
DataReader.GetRecords.GetAllRecords(request, response);
return response;
}
Is it wrong that i have the entire DAL and many Helper methods as static classes and methods? If so, why?
As a result of using Static method you can access instance variables like properties and fields this means you need to pass the request to every method.
If you used instances of GetRecords you could do somehting liek
GetRecords gr = new gr();
gr.Request = request;
gr.VerifyMandatoryParameters();
gr.SetDefaultValues();
gr.SetDefaultResponseValues(response);
gr.GetAllRecords();
Also if you implment a fluent interfaces you could have written it like this.
GetRecords gr = new gr();
gr.SetRequest(request)
.VerifyMandatoryParameters()
.SetDefaultValues()
.SetDefaultResponseValues(response)
.GetAllRecords();
But there's nothing "wrong" with what you did
This is subjective matter, but I think that static methods are evil in general (except very few situations).
Here is a link to a talk discussing why this is a problem: http://misko.hevery.com/2008/11/11/clean-code-talks-dependency-injection/
Although the speaker is talking about Java, but it is quite similar language, and static methods are being used almost the same way in both languages.
In short, Yes.
I imagine that you are performing few to no unit tests against your code. If you plan on adding any, at any point; a statically accessed DAL will cause you no end of grief (I am dealing this exact scenario right now).
Instead, you should be passing in, say a DataAccessService interface, that can be mocked for testing, and implemented pointing to your actual data store in production.
So, I would expect your code to look more like:
public GetRecordsResponse GetRecords(GetRecordsRequest request, DataAccessService dataAccess)
{
var response = new GetRecordsResponse();
Util.GetRecords.VerifyMandatoryParameters(request);
Util.GetRecords.SetDefaultValues(request);
Util.GetRecords.SetDefaultResponseValues(request, response);
dataAccess.GetAllRecords(request, response);
return response;
}
精彩评论