code you don't write by hand is code you don't have to think about. adding new factories and serializers is tedious and error-prone. hand-written also means if you need to change something it's even MORE tedious and error-prone.
there is a fine line between not fixing what ain't broke and not refactoring at all. refusing to make improvements to clean up something that has grown crufty and an impediment to understanding the code base is bad. (this is why I have been adding tests to the code base: tests allow you to change code but be confident that you didn't break things.) when you started with half a dozen message factories writing them by hand was fine. now we have 21 verbhandler types, all (?) of which have hand-written factories. I don't think refactoring this is premature anymore. all that said, you are probably right that thrift is a poor fit here. On Sat, Mar 28, 2009 at 10:34 AM, Avinash Lakshman <[email protected]> wrote: > Why is it ad-hoc? And it uses a factory pattern and I don't think it hard > once you get a hang of it. Consumers of the system are not even going to > know about these details. Personally I am never a fan of fixing anything > that is not broken - in this case it has been working really well for us. > This is now just a matter of what one might prefer. Thrift is something that > I would not like to see anywhere apart from the entry point. With regards to > the using the string to lookup the handlers it was done because if you don't > do that then you will have to resort to RPC style instead of message passing > or find the handlers based on the kind of messages i.e if-else branching. We > use non-blocking I/O for all our internal messaging and Thrift using > blocking I/O. There is big difference in throughput and also Thrift > non-blocking I/O from what I hear is horrendous in performance and > stability. My friend you don't have my vote for this :). > Avinash > > On Sat, Mar 28, 2009 at 8:11 AM, Jonathan Ellis <[email protected]> wrote: > >> we have a Message class that mostly represents a bunch of bytes (but >> sometimes does not, which in some cases causes bugs) and a bunch of >> other *Message classes that are not Message subclasses but generate >> Message objects (so you have the amusingly redundant Message message = >> readResponseMessage.makeReadResponseMessage() in places). >> >> I think we can replace these ad-hoc and tedious-to-write Message >> factories with generated thrift code. Thrift is good at this and >> efficient (currently our message identifiers are inefficient strings >> like "ROW-READ-VERB-HANDLER"). >> >> Any objections to investigating replacing the hand-coded messages with >> thrift? >> >
