From: Hiroo H. <hir...@co...> - 2004-08-21 21:07:18
|
Hi Rib, On Sat, 21 Aug 2004 11:55:26 -0700 Rib Rdb <ri...@gm...> wrote: Rib> > My next question would be why Converter still extended the Driver:-) Rib> > The checksumming is the only reason? Or did you see any other problems? Rib> Rib> I didn't see any problems at the time, but I just thought of a problem Rib> when I saw that you added dissect() to IPatch. If Converter is not an Rib> IPatchDriver, so there would be no way for a Patch's driver to be a Rib> converter. I think instead we need to make dissect(byte[]) and Rib> dissect(SysexMessage[]), or else make createPatch return IPatch[] and Rib> move createPatch from IPatchDriver to IDriver. What do you think? I was thinking about the problem, too Your mail made the problem clear. My take is almost the latter. Let me explain what I'm thinking now. IConverter is used by Patch.dissect() now. IPatch.dissect() is called by ImportAllDialog.doImport(), ImportMidiFile.doImport(), LibraryFrame.importPatch(), SceneFrame.importPatch(), and SysexGetDialog.pasterInfoSelectedFrame(). The input data for SysexGetDialog.pasterInfoSelectedFrame() is SysexMessage[] and driver is known. What we need is IPatch[] IPatchDriver.createPatch(SysexMessage[]) The most of code in SysexGetDialog.pasterInfoSelectedFrame() will be included in this method. The current pasterInfoSelectedFrame() tries other device/drivers when the received data is not expected one. I think we should simply cause error on this case. The input data for ImportAllDialog.doImport(), ImportMidiFile.doImport(), LibraryFrame.importPatch(), and SceneFrame.importPatch() is byte[] and driver is unknown for them. I think this is the job of patch factory you wrote. static IPatch[] Patch.createPatch(byte[]) This static method will be used for any IPatch object. Using Patch class (an implementation of IPatch) may not be proper. On the other hand I feel it might be overkill to create a new class (PatchFactory.java?) only for this, and I don't see a proper class which will include this... This method will use IDriver.supportsPatch(String, IPatch) (we may want to change the second argument to byte[]) and the selected driver will use IPatch[] IDriver.createPatch(byte[]) which you proposed. And if we have IDriver.supportsPatch(String, byte[]), a Patch's driver does not has to be a converter. IPatch IPatchDriver.createPatch(byte[]) will be replaced by IPatch[] IDriver.createPatch(byte[]). IPatch.dissect() is no longer an interface method. How do you think? If you agree with me and don't mind, I'll try to make this change during this weekend. PS. To make consistent with IPatch IPatchDriver.createPatch(byte[]), Patch IPatchDriver.createNewPatch() can be renamed to IPatch IPatchDriver.createPatch(). Of course Patch Driver.createNewPatch() will stay the same. -- Hiroo Hayashi |