Skip to content

Commit

Permalink
Inline newGrpcPeerFromURL
Browse files Browse the repository at this point in the history
  • Loading branch information
@akshayjshah
akshayjshah committed Jan 28, 2023
1 parent 30570f4 commit 5e211a7
Showing 1 changed file with 5 additions and 9 deletions.
14 changes: 5 additions & 9 deletions protocol_grpc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"math"
"net/http"
"net/textproto"
"net/url"
"runtime"
"strconv"
"strings"
Expand Down Expand Up @@ -104,10 +103,14 @@ func (g *protocolGRPC) NewClient(params *protocolClientParams) (protocolClient,
if err != nil {
return nil, err
}
peer := newPeerFromURL(url, ProtocolGRPC)
if g.web {
peer = newPeerFromURL(url, ProtocolGRPCWeb)
}
return &grpcClient{
protocolClientParams: *params,
web: g.web,
peer: newGrpcPeerFromURL(url, g.web),
peer: peer,
}, nil
}

Expand Down Expand Up @@ -220,13 +223,6 @@ type grpcClient struct {
peer Peer
}

func newGrpcPeerFromURL(url *url.URL, web bool) Peer {
if web {
return newPeerFromURL(url, ProtocolGRPCWeb)
}
return newPeerFromURL(url, ProtocolGRPC)
}

func (g *grpcClient) Peer() Peer {
return g.peer
}
Expand Down

2 comments on commit 5e211a7

@mattrobenolt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I avoided this to avoid doing the unnecessary work inside newPeerFromURL twice, but I guess it's basically free now that it's not doing url.Parse.

I guess an option could be:

	peer := newPeerFromURL(url, ProtocolGRPC)
	if g.web {
		peer.Protocol = ProtocolGRPCWeb
	}

or

protocol := ProtocolGRPC
if g.web {
	protocol = ProtocolGRPCWeb
}
peer := newPeerFromURL(url, protocol)

But this is fine anyways.

@mattrobenolt
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd guess at this point, the compiler would very likely inline this function call now so shouldn't matter at all.

Please sign in to comment.