I have a web service (web broker) that works fine and each request is sent to a corresponding object that relates to the request and the response is returned, for example the /customerinfo request (GET) calls an object that is private to the TWebModule that returns customer information.
begin
Response.Content := FCustomers.CustomerInfo(Request);
end;
BTW, this is not the actual code, but is just an example. This works fine and has been working fine for some time. However, another developer has come up with what they consider a better way fo doing this. First, they add a property of TWebRequest to all objects that use TWebRequest, so in this example the FCustomer object has a WebRequest property. Then in the BeforeDispatch set the WebRequest property of all the web module objects (not just the one that relates to the request) to the web request. The advantage of this is so that you do not need to pass in the WebRequest in the action e.g.
procedure TMyWebModule.CustomerInfoAction(Sender: TObject; Request: TWebRequest; Response: TWebResponse; var Handled: Boolean);
begin
Response.Content := FCustomers.CustomerInfo;
end;
They have also proposed that we could also set the TWebResponse on all the objects to that then all you need to do is this.
procedure TMyWebModule.CustomerInfoAction(Sender: TObject; Request: TWebRequest; Response: TWebResponse; var Handled: Boolean);
begin
FCustomers.CustomerInfo;
end;
The other change that is being proposed is to add validation to the TWebRequest, by having a new class that descends from TWebRequest, lets call it TMyWebRequest and this will have validation code, so that you can call something like MyWebRequest.Validate it will do some validation. Currently there is a validation class that takes the TWebRequest, but this would be removed and the TMyWebRequest would validate itself.
In the AfterDispatch method it will iterate through all the objects and set the MyWebRequest property to nil.
There are a few things I am not keen about with making these changes.
- I don't like the idea of descending from TWebRequest just to add some validation methods, and much prefer having validation classes that are responsible for doing the validation.
- Currently passing TWebRequest makes it clear and explicit that the parameter is expected, making it easier to trace.
- The current way keeps the responsibility of handling the request and response with the action.
- It is setting the MyWebRequest property of all the objects, even though it is only required for the object that will be called, for example it sets the MyWebRequest property on the Order, Product, System and Customer objects even though it is only the Customer object that will be called with that requrest.
- It means that a developer can use both methods, this could cause confusion.
- It will take some time to change all the services to work this new way.
The benefits I can see from doing this are:
- Reduce the number of parameters passed.
My vote would be to keep it simple and only do the work necessary to handle the request, pass the parameters. Just because something is convenient (to the developer) shouldn't make it a candidate for refactoring. For the validation object, again I would keep it its own object (following the SOLID principles). This will make it easier to test in isolation without adding unnecessary dependencies which can be a time suck when the problem is in the added dependency.
ReplyDeleteThanks for the comment and I agree with what you say.
Delete