search

bitcoinj / bitcoinj

A library for working with Bitcoin
https://bitcoinj.org
Apache License 2.0
4.96k stars 2.48k forks source link

ProtobufParser is misleading #1057

Closed ollekullberg closed 9 years ago

ollekullberg commented 9 years ago

After working with ProtobufParser for 1/2 a year (we use it in the Stroem implementation) I find the name misleading. A name change would be breaking, but I still request it.

The responsibility of PP is to read raw data and chunk it so that complete protobuf messages comes out. The name is probably a twist on its super class: StreamParser, but PP also has a "write()" method, which indicates that it also can send, not only parse. I see PP as the main gateway to the network; when I call "write()" the message will be sent to the server, and I don't have to care much.

A name, something along the lines of "ProtobufConnection","NetworkProtobufConnection" or even "ProtobufEnabledNetworkConnection"? The "Connection" suffix would relate it to "PaymentChannelClientConnection", from where PP is used.

Since I'm using this class a lot, I can create a PR to rename it, if we agree on a new name that is.

I've also made a class diagram of bitcoinj payment channel core classes in yEd. Don't know if you are interested?

paymenchannelclassdiagram

ollekullberg commented 9 years ago

Ehh, sorry. That pic was old. This is a newer one:

paymenchannelclassdiagram

mikehearn commented 9 years ago

ProtobufConnection sounds good to me. Happy to accept a patch for this.