zookeeper-bookkeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ivan Kelly <iv...@yahoo-inc.com>
Subject Review of public APIs
Date Wed, 19 Oct 2011 15:40:31 GMT
I've just gone through the public apis for BK and HW and have found the following issues. Most
of them are issues with protection being too loose. There's only 2-3 breaks here which should
require code change on the users' part. The rest are things that they really shouldn't be
using in any case.

BookKeeper#createLedger, parameter is named passwd, "Key" used in LedgerHandle api
BookKeeper#getBookieClient shouldn't be public
BookKeeper#createComplete shouldn't be public
BookKeeper#openComplete shouldn't be public
BookKeeper#deleteComplete shouldn't be public
BookKeeper#halt could be changed to close(), should throw a BKException

LedgerHandle#getLedgerKey passwd is used in BookKeeper, should possibly be private
LedgerHandle#getLedgerMetadata shouldn't be public
LedgerHandle#getDigestManager shouldn't be public
LedgerHandle#getDistributionSchedule shouldn't be public
LedgerHandle#writeLedgerConfig shouldn't be public
LedgerHandle#addEntry should return void, errors should go in an Exception
LedgerHandle#readComplete should not be public
LedgerHandle#addComplete should not be public
LedgerHandle#readLastConfirmedCompelte should not be public
LedgerHandle#closeComplete should not be public

ASyncCallback#RecoverCallback shouldn't be public

HedwigClient#getSslFactory shouldn't be public
HedwigClient#getConsumeCallback shouldn't be public
HedwigClient#doConnect shouldn't be public
HedwigClient#getHostFromChannel shouldn't be public
HedwigClient#getResponseHandlerFromChannel shouldn't be public
HedwigClient#getHostForTopic shouldn't be public
HedwigClient#clearAllTopicsForHost shouldn't be public
HedwigClient#getClientTimer shoulnd't be public
HedwigClient#stop should throw some sort of Exception in the case of errors

HedwigPublisher#publish shouldn't use protobuf ByteString, as it requires the user to import
protobufs
HedwigPublisher#getChannelForHost shouldn't be public

HedwigSubscriber#HedwigSubscriber shouldn't be public
HedwigSubscriber#doConsume shouldn't be public
HedwigSubscriber#hasSubscription probably shouldn't be public
HedwigSubscriber#getSubscriptionList shoulnd't exist
HedwigSubscriber#getChannelForTopic shouldn't be public
HedwigSubscriber#setChannelforTopic shouldn't be public
HedwigSubscriber#removeChannelForTopic shound't be public

MessageHandler#consume should be called 'deliver'

The hedwig client is under a netty package. There's nothing netty specific about the api,
so it should be in the org.apache.hedwig.client package. 

In both BK and HW, context objects are passed around with callbacks. I don't think this is
necessary, but it doesn't bug me too much. 


-Ivan
Mime
View raw message