nrpc
nrpc copied to clipboard
Use pointers for Server/Client args and return values
Returning a pointer is nice because if the function has an error most times you do not want to return an empty response struct
Below is an example of how gRPC func signatures look
type routeGuideServer struct {
...
}
...
func (s *routeGuideServer) GetFeature(ctx context.Context, point *pb.Point) (*pb.Feature, error) {
...
}
...
func (s *routeGuideServer) ListFeatures(rect *pb.Rectangle, stream pb.RouteGuide_ListFeaturesServer) error {
...
}
...
func (s *routeGuideServer) RecordRoute(stream pb.RouteGuide_RecordRouteServer) error {
...
}
...
func (s *routeGuideServer) RouteChat(stream pb.RouteGuide_RouteChatServer) error {
...
}
Plus all of the get methods on the gRPC generated types are only defined on pointer receivers.
Makes sense. @mdevan do you agree ? If so we would welcome a PR.
What about the service/method subject params? They should probably remain plain strings.
Agree that other args/receivers should be pointers.
Subject params should remain strings indeed.
IMO it is not as simple as it sounds especially on the return side.
Pure NATS client calling convention results in client like this (from my code):
func (c *FinClient) BookUpdate(req *Book) (resp BookResponse, err error) {
err = c.ec.Request(BookUpdateSubj, req, &resp, c.timeout)
return
}
Note, that NATS takes pointer to a struct to fill for the response object. That means someone has to allocate it. If caller is to allocate the struct, then response can only be returned as input argument.
So, there is a choice to make:
- Use idiomatic calling convention and return a struct.
- Take return value as a pointer just like NATS does.
The way nats EncodedConn handle return types is required because Request is generic and there is no other way for it to allocate the response itself.
In nrpc we generate the code, so we can generate code the allocate the response easily.
Passing the return value as a pointer has 2 cons for the developer:
- Declare the response variable for each request
- Always know the exact response type before calling the function
So the choice remains:
- Return plain structure
- Return pointer to structure
I think returning pointers has only advantages and will have little impact for the existing code.
Taking pointers for the arguments on the other hand, will have consequences for the existing code, so we could make this behavior optional (or have an option to keep the old behavior)
All protobuf messages must be passed and returned by reference - protobuf implementation requires that by making the messages (indirectly) contain pragma.DoNotCopy. That does not enforce by-reference passing, but is caught by go vet
This is a very good point
It's been a while but it is now implemented