Menu
▾
▴
jsynthlib-devel
|
From: Joe E. <jo...@em...> - 2006-02-22 08:44:55
Attachments:
smime.p7s
|
As the first step toward getting the classes refactored into a sane package hierarchy, I have committed my changes to support saving patches to XML. Here's the rundown: o For the time being, I'm using the jdom.jar library.... so you'll need to add jdom.jar to your classpath. Just look for any place where you have "groovy.jar" and change it to either "groovy.jar; jdom.jar" or "groovy.jar:jdom.jar". I don't remember if the "colon vs. semicolon" thing is platform-dependent. o Whenever you save any library (*just* libraries.... I haven't done scenes, yet), they are saved as the original "something.patchlib" file as well as "something.patchlib.xml". o When you load a file, you'll still be loading the original ".patchlib" versions of your library. To change this, you need to open AbstractLibraryFrame and look for "void open(File file)". In that method, you'll see two boolean values: "readXMLFile" and "readOldFile". IF YOU MAKE BOTH OF THESE TRUE, YOU'LL GET TWO COPIES OF EACH PATCH! I did this so that you can load both versions at the same time and then open them both in a patch editor and see if there is any difference. Just be warned, however, that if you *save* the library after that, both the .patchlib and the .patchlib.xml will now have duplicates. So, if you load, save, load, save, etc. with both booleans set to "true", your library will double each time, and it will get really big really fast. I learned this the hard way. :) o The patch data is compressed via gzip and then converted to base64 and then saved in the XML file. As a result, I've seen the XML file be anywhere from 10% to 50% the size of the .patchlib. ==== Things to think about while trying this ==== o Right now, so that the XML file is closely associated with the .patchlib file, I've named it .patchlib.xml. What extension do we want to have permanently? - Joe |
|
From: Rib R. <ri...@gm...> - 2006-03-07 09:23:48
|
I just glanced at the code. It looks like you're casting everything
in the PatchBasket to Patch. That won't work with the XML driver (or
any other custom Patch classes). It looks like a simple solution for
doing the output would be to make IPatch extend
org.jsynthlib.utils.Writable, and use org.jsynthlib.utils.XMLWriter
instead of the JDOM XMLOutputter. Then each patch implementation only
needs to add a write method.
Reading the XML files back in is more complicated, because the parser
needs to know what it should do with the elements it gets (either call
a setter method with the string contents, or build an object, then
call a setter). This probably means we need to decide on some
restrictions on how patches can store themselves. Perhaps something
like:
<patchlib type=3D"scene, library, etc">
<patch type=3D"classname">
<author>Whatever</author>
<!-- and other string values as determined by the implementation -->
<!-- or sub patches for bank patches -->
</patch>
</patchlib>
On 2/22/06, Joe Emenaker <jo...@em...> wrote:
> As the first step toward getting the classes refactored into a sane
> package hierarchy, I have committed my changes to support saving patches
> to XML.
>
> Here's the rundown:
> o For the time being, I'm using the jdom.jar library.... so you'll need
> to add jdom.jar to your classpath. Just look for any place where you
> have "groovy.jar" and change it to either "groovy.jar; jdom.jar" or
> "groovy.jar:jdom.jar". I don't remember if the "colon vs. semicolon"
> thing is platform-dependent.
>
> o Whenever you save any library (*just* libraries.... I haven't done
> scenes, yet), they are saved as the original "something.patchlib" file
> as well as "something.patchlib.xml".
>
> o When you load a file, you'll still be loading the original
> ".patchlib" versions of your library. To change this, you need to open
> AbstractLibraryFrame and look for "void open(File file)". In that
> method, you'll see two boolean values: "readXMLFile" and "readOldFile".
> IF YOU MAKE BOTH OF THESE TRUE, YOU'LL GET TWO COPIES OF EACH PATCH! I
> did this so that you can load both versions at the same time and then
> open them both in a patch editor and see if there is any difference.
> Just be warned, however, that if you *save* the library after that, both
> the .patchlib and the .patchlib.xml will now have duplicates. So, if you
> load, save, load, save, etc. with both booleans set to "true", your
> library will double each time, and it will get really big really fast. I
> learned this the hard way. :)
>
> o The patch data is compressed via gzip and then converted to base64
> and then saved in the XML file. As a result, I've seen the XML file be
> anywhere from 10% to 50% the size of the .patchlib.
>
> =3D=3D=3D=3D Things to think about while trying this =3D=3D=3D=3D
>
> o Right now, so that the XML file is closely associated with the
> .patchlib file, I've named it .patchlib.xml. What extension do we want
> to have permanently?
>
> - Joe
>
>
>
|
|
From: Joe E. <jo...@em...> - 2006-03-07 10:13:48
Attachments:
smime.p7s
|
Rib Rdb wrote:
> I just glanced at the code. It looks like you're casting everything
> in the PatchBasket to Patch.
That was before I realized that SceneLibrary was also saving through the
same code. So, at the very least, I have to check if it's an instance of
a Patch or a Scene.
> That won't work with the XML driver (or
> any other custom Patch classes). It looks like a simple solution for
> doing the output would be to make IPatch extend
> org.jsynthlib.utils.Writable, and use org.jsynthlib.utils.XMLWriter
> instead of the JDOM XMLOutputter. Then each patch implementation only
> needs to add a write method.
>
> Reading the XML files back in is more complicated, because the parser
> needs to know what it should do with the elements it gets
This was my first misgiving about having each patch implementation
define a write method. Your options for reading are:
1 - Have the code for reading the XML be in different classes from the
code for writing them (very, very bad form, in my opinion), or
2 - Have each patch implementation define a read method as well.
The problem I have with #2 is that it seems like we'd end up using it
much like we find the driver for a sysex. We just give it to each driver
and ask it if it recognizes it. I'd like to avoid that for patch
implementations.
> (either call
> a setter method with the string contents, or build an object, then
> call a setter). This probably means we need to decide on some
> restrictions on how patches can store themselves.
Well, here's another thought I had. Right now, Patch has some fields
named Author and Date, I think. However, they're no longer used to hold
author and date. They're called "Field 1" and "Field 2" in the
LibraryFrame. We *could* just allow any number of name/value pairs (ie
"String getValue(String name)" and "void setValue(String name, String
value)"), and then the XML code would have no trouble at all retrieving
them and setting them. How the individual classes chose to use them
would be up to them. If we wanted some tighter controls, we could allow
Patch subclasses to define an array of allowed value names (ie
"allowedNames = new String[] {'Author', 'Date', 'Comment', 'FromPatch',
'FromBank' ") and we could trigger an ErrMsg if the XML file contained a
non-allowed value name.
To downside to all of this is that it would be a burden for the Patch
subclasses to access their own data, since they'd probably always have
to use their own getValue and setValue methods. Also, dealing with any
data other than String would be a bother.
An alternative would be to have the XML code use inspection to find all
matched getter/setters, discover the data type, and magically
save/restore them. The downsides to this would be:
1 - The XML code becomes significantly more complex and error-prone
2 - We would have to decide on how to have the XML code cope if it
encounters a getter/setter that it doesn't know how to save (like if a
getter returns a Vector, or Hashtable, say).
3 - Anybody who subclasses Patch would have to remember to make public
getter *and* setter methods for any new data elements that they want
saved. If they *forget* to do so, this could lead to a "why isn't my
field getting saved?" problem that is tough to track down.
- Joe
|
|
From: Rib R. <ri...@gm...> - 2006-03-07 20:21:11
|
Okay. Let me explain a little about how the XML parser I built works.
The basic idea is that has a list of element names which can have
child elements. When it encounters the start tag for one of these
elements, it starts building an object.
So if there was an xml file like this:
<patchbasket>
<patch>
<author>foo</author>
</patch>
</patchbasket>
The parser would:
1) Create a patchbasket object
2) create a patch object
3) Use introspection to find (and call) a setter (in the patch)
setAuthor(String) or addAuthor(String)
4) Use introspection to find (and call) a setter in the basket class
setPatch or addPatch which will accept the created patch
5) Return the patchbasket object
So to set up the parser we need to tell it:
1) Which element should cause it to start building an object
2) How to create those objects
With that foundation, my recommendation (which I explained terribly in
my last email) is to allow two generating elements: patchbasket and
patch. Any other element would be treated as a string property by the
parser. Then each IPatch implementation has to have some sort of a
write method, plus matching setters. We should probably have some
sort of helper for binary data. The implementation would look
somthing like this:
void write(XMLWriter xml) {
xml.writeProperty("author", self.author);
xml.writeBinaryProperty("data", self.sysex);
}
void setAuthor(String author) { /* ... */ }
void setData(String data) { self.sysex =3D XMLTools.decodeBinary(data); }
On 3/7/06, Joe Emenaker <jo...@em...> wrote:
> Rib Rdb wrote:
> > I just glanced at the code. It looks like you're casting everything
> > in the PatchBasket to Patch.
> That was before I realized that SceneLibrary was also saving through the
> same code. So, at the very least, I have to check if it's an instance of
> a Patch or a Scene.
> > That won't work with the XML driver (or
> > any other custom Patch classes). It looks like a simple solution for
> > doing the output would be to make IPatch extend
> > org.jsynthlib.utils.Writable, and use org.jsynthlib.utils.XMLWriter
> > instead of the JDOM XMLOutputter. Then each patch implementation only
> > needs to add a write method.
> >
> > Reading the XML files back in is more complicated, because the parser
> > needs to know what it should do with the elements it gets
> This was my first misgiving about having each patch implementation
> define a write method. Your options for reading are:
> 1 - Have the code for reading the XML be in different classes from the
> code for writing them (very, very bad form, in my opinion), or
> 2 - Have each patch implementation define a read method as well.
>
> The problem I have with #2 is that it seems like we'd end up using it
> much like we find the driver for a sysex. We just give it to each driver
> and ask it if it recognizes it. I'd like to avoid that for patch
> implementations.
> > (either call
> > a setter method with the string contents, or build an object, then
> > call a setter). This probably means we need to decide on some
> > restrictions on how patches can store themselves.
> Well, here's another thought I had. Right now, Patch has some fields
> named Author and Date, I think. However, they're no longer used to hold
> author and date. They're called "Field 1" and "Field 2" in the
> LibraryFrame. We *could* just allow any number of name/value pairs (ie
> "String getValue(String name)" and "void setValue(String name, String
> value)"), and then the XML code would have no trouble at all retrieving
> them and setting them. How the individual classes chose to use them
> would be up to them. If we wanted some tighter controls, we could allow
> Patch subclasses to define an array of allowed value names (ie
> "allowedNames =3D new String[] {'Author', 'Date', 'Comment', 'FromPatch',
> 'FromBank' ") and we could trigger an ErrMsg if the XML file contained a
> non-allowed value name.
>
> To downside to all of this is that it would be a burden for the Patch
> subclasses to access their own data, since they'd probably always have
> to use their own getValue and setValue methods. Also, dealing with any
> data other than String would be a bother.
>
> An alternative would be to have the XML code use inspection to find all
> matched getter/setters, discover the data type, and magically
> save/restore them. The downsides to this would be:
> 1 - The XML code becomes significantly more complex and error-prone
> 2 - We would have to decide on how to have the XML code cope if it
> encounters a getter/setter that it doesn't know how to save (like if a
> getter returns a Vector, or Hashtable, say).
> 3 - Anybody who subclasses Patch would have to remember to make public
> getter *and* setter methods for any new data elements that they want
> saved. If they *forget* to do so, this could lead to a "why isn't my
> field getting saved?" problem that is tough to track down.
>
> - Joe
>
>
>
|
|
From: Joe E. <jo...@em...> - 2006-03-07 23:30:46
Attachments:
smime.p7s
|
Rib Rdb wrote:
> Okay. Let me explain a little about how the XML parser I built works.
> The basic idea is that has a list of element names which can have
> child elements. When it encounters the start tag for one of these
> elements, it starts building an object.
> So if there was an xml file like this:
>
> <patchbasket>
> <patch>
> <author>foo</author>
> </patch>
> </patchbasket>
>
There's some kind of patch data included, I hope. :) Sysex, maybe?
> The parser would:
> 1) Create a patchbasket object
> 2) create a patch object
> 3) Use introspection to find (and call) a setter (in the patch)
> setAuthor(String) or addAuthor(String)
> 4) Use introspection to find (and call) a setter in the basket class
> setPatch or addPatch which will accept the created patch
> 5) Return the patchbasket object
>
Okay... this looks good.
> Then each IPatch implementation has to have some sort of a
> write method, plus matching setters. We should probably have some
> sort of helper for binary data. The implementation would look
> somthing like this:
>
> void write(XMLWriter xml) {
> xml.writeProperty("author", self.author);
> xml.writeBinaryProperty("data", self.sysex);
> }
>
> void setAuthor(String author) { /* ... */ }
> void setData(String data) { self.sysex = XMLTools.decodeBinary(data); }
>
I'm a little uneasy by the requirement that the "author" in
writeProperty has to match the "Author" in "setAuthor". First, you have
to have two dissimilar items (a hard-coded string "something" in a call
to writeProperty and a method called "setSomething") that have to match.
Furthermore, if they don't match, your first indication of a problem
comes *after* the XML file has been written and you're now trying to
load it.
The second problem is even worse, though. If *you* write an
implementation of IPatch... then it would have a write() method. Now,
suppose that *I* come along and write a subclass your implementation. If
I want the additional data of my subclass saved, then I also have to
have a write() method. However, if I do that, then *your* write method
doesn't get called anymore. I'd either have to go look at your class and
save all of the data elements that *your* write() method was saving
(imagine what happens when you add/remove some data elements to your
class without telling me), or I'd have to call "super.write(xml)" any
time I wrote a write() method of my own.
Either of those solutions would require that the author of the subclass
have an awareness of the inner workings of the patch-saving system that
I'm not convinced should be necessary.
I think we can find a slicker way.
- Joe
|
|
From: Rib R. <ri...@gm...> - 2006-03-08 04:38:58
|
On 3/7/06, Joe Emenaker <jo...@em...> wrote:
> Rib Rdb wrote:
> > Okay. Let me explain a little about how the XML parser I built works.
> > The basic idea is that has a list of element names which can have
> > child elements. When it encounters the start tag for one of these
> > elements, it starts building an object.
> > So if there was an xml file like this:
> >
> > <patchbasket>
> > <patch>
> > <author>foo</author>
> > </patch>
> > </patchbasket>
> >
> There's some kind of patch data included, I hope. :) Sysex, maybe?
Of course. I was keeping it a short as possible since this is just an
example of how the parser works.
> > The parser would:
> > 1) Create a patchbasket object
> > 2) create a patch object
> > 3) Use introspection to find (and call) a setter (in the patch)
> > setAuthor(String) or addAuthor(String)
> > 4) Use introspection to find (and call) a setter in the basket class
> > setPatch or addPatch which will accept the created patch
> > 5) Return the patchbasket object
> >
> Okay... this looks good.
> > Then each IPatch implementation has to have some sort of a
> > write method, plus matching setters. We should probably have some
> > sort of helper for binary data. The implementation would look
> > somthing like this:
> >
> > void write(XMLWriter xml) {
> > xml.writeProperty("author", self.author);
> > xml.writeBinaryProperty("data", self.sysex);
> > }
> >
> > void setAuthor(String author) { /* ... */ }
> > void setData(String data) { self.sysex =3D XMLTools.decodeBinary(data);=
}
> >
> I'm a little uneasy by the requirement that the "author" in
> writeProperty has to match the "Author" in "setAuthor". First, you have
> to have two dissimilar items (a hard-coded string "something" in a call
> to writeProperty and a method called "setSomething") that have to match.
> Furthermore, if they don't match, your first indication of a problem
> comes *after* the XML file has been written and you're now trying to
> load it.
>
> The second problem is even worse, though. If *you* write an
> implementation of IPatch... then it would have a write() method. Now,
> suppose that *I* come along and write a subclass your implementation. If
> I want the additional data of my subclass saved, then I also have to
> have a write() method. However, if I do that, then *your* write method
> doesn't get called anymore. I'd either have to go look at your class and
> save all of the data elements that *your* write() method was saving
> (imagine what happens when you add/remove some data elements to your
> class without telling me), or I'd have to call "super.write(xml)" any
> time I wrote a write() method of my own.
>
> Either of those solutions would require that the author of the subclass
> have an awareness of the inner workings of the patch-saving system that
> I'm not convinced should be necessary.
How is requiring a call to the super method an unreasonable awareness
of the inner workings? Isn't that sort of a standard practice?
Also, subclassing IPatch is something that is not done often and is
complicated. It already requires reading documentation to do it
properly. So we can just add to the documentation explaining how the
write method has to work with the setters.
> I think we can find a slicker way.
Do you have any suggestions. I think using introspection to find
which attributes to store would be more confusing to a driver author,
and very tricky to do correctly.
This is just the simplest solution I could think of without writing a
new parser.
|
|
From: Joe E. <jo...@em...> - 2006-03-08 07:48:53
Attachments:
smime.p7s
|
Rib Rdb wrote: > On 3/7/06, Joe Emenaker <jo...@em...> wrote: > >> Either of those solutions would require that the author of the subclass >> have an awareness of the inner workings of the patch-saving system that >> I'm not convinced should be necessary. >> > > How is requiring a call to the super method an unreasonable awareness > of the inner workings? I didn't say that it was unreasonable... I just said that I'm not convinced that it should be necessary. The biggest problem I see is that, if you forget to call the super.write() method in your new class, then all of the superclass data isn't saved... and you don't discover it until you've closed the patchbasket and tried to reload it again.... at which point, the data is not coming back. In most other cases where you have to explicitly use the super prefix, its in the constructor, and forgetting to do so is usually immediately evident (due to thrown exceptions or a JFrame not actually getting instantiated... etc). Now, granted this is a problem that should only affect someone who's authoring a subclass to a implementation of IPatch.... but the combination of how easy it would be to forget to call super.write() and the consequences for doing so... well... they worry me. >> I think we can find a slicker way. >> > Do you have any suggestions. I think using introspection to find > which attributes to store would be more confusing to a driver author, > and very tricky to do correctly. > I'm not seeing how a driver author needs to be bothered with it. My rough idea of the introspection would work something like this: - Get a list of all public getters and setters for the object and make a list that combines just the attributes that appear in both lists (so, the XML writer would then know all of the attributes that we can save now, and also set later in the object at the time we read the XML file). - If any of those getters return anything other than String (or any other data types we know how to save and restore), then throw an exception, or fail, or complain, or do something. - Get the data from the getters and save them to the XML. The benefits here are that, if you subclass an implementation of IPatch, it's harder to accidentally disrupt the saving of the data in the superclass. In fact, if the superclass declares its data, getters, and setters as final, then it might be *impossible* for you to mess it up. The downside is that the saving of the data just kinda happens "like magic". Once the author makes a public getter and setter for some piece of data, then it magically starts getting saved. In that respect, it's almost like it would be inventing an XML version of serialization. > This is just the simplest solution I could think of without writing a > new parser. Well, I'm all for that. In fact, our two proposals don't seem to differ in regards to how the XML gets read, parsed, and sets the values in the objects. I think we only differ on how we generate the XML at save time, no? - Joe |
|
From: Rib R. <ri...@gm...> - 2006-03-08 17:36:30
|
On 3/7/06, Joe Emenaker <jo...@em...> wrote:
> Rib Rdb wrote:
> > On 3/7/06, Joe Emenaker <jo...@em...> wrote:
> >
> >> Either of those solutions would require that the author of the subclas=
s
> >> have an awareness of the inner workings of the patch-saving system tha=
t
> >> I'm not convinced should be necessary.
> >>
> >
> > How is requiring a call to the super method an unreasonable awareness
> > of the inner workings?
> I didn't say that it was unreasonable... I just said that I'm not
> convinced that it should be necessary.
>
> The biggest problem I see is that, if you forget to call the
> super.write() method in your new class, then all of the superclass data
> isn't saved... and you don't discover it until you've closed the
> patchbasket and tried to reload it again.... at which point, the data is
> not coming back. In most other cases where you have to explicitly use
> the super prefix, its in the constructor, and forgetting to do so is
> usually immediately evident (due to thrown exceptions or a JFrame not
> actually getting instantiated... etc).
>
> Now, granted this is a problem that should only affect someone who's
> authoring a subclass to a implementation of IPatch.... but the
> combination of how easy it would be to forget to call super.write() and
> the consequences for doing so... well... they worry me.
Any problem with serialization, no matter how it happens is going to
cause problems until try to load it. By making it happen explicitly,
at least the author should know to look for problems there.
> >> I think we can find a slicker way.
> >>
> > Do you have any suggestions. I think using introspection to find
> > which attributes to store would be more confusing to a driver author,
> > and very tricky to do correctly.
> >
> I'm not seeing how a driver author needs to be bothered with it.
Sorry, I meant patch author.
> My rough idea of the introspection would work something like this:
> - Get a list of all public getters and setters for the object and make a
> list that combines just the attributes that appear in both lists (so,
> the XML writer would then know all of the attributes that we can save
> now, and also set later in the object at the time we read the XML file).
> - If any of those getters return anything other than String (or any
> other data types we know how to save and restore), then throw an
> exception, or fail, or complain, or do something.
> - Get the data from the getters and save them to the XML.
>
> The benefits here are that, if you subclass an implementation of IPatch,
> it's harder to accidentally disrupt the saving of the data in the
> superclass. In fact, if the superclass declares its data, getters, and
> setters as final, then it might be *impossible* for you to mess it up.
>
> The downside is that the saving of the data just kinda happens "like
> magic". Once the author makes a public getter and setter for some piece
> of data, then it magically starts getting saved. In that respect, it's
> almost like it would be inventing an XML version of serialization.
This has several problems. For one, the XML setters are not normal
setters: they have to take a string argument. Someone who just writes
a patch class without knowing about serialization is probably not
going to write this type of setter. I would think they would have a
harder time trying to find the problem than someone who write a write
method that didn't call super; at least this person would know the
problem is in their write method.
Also, you lose some flexibility. For example the write method could
do something like this
if (xml.useGzip())
xml.writeGzippedProperty('gzippedSysex', this.sysex)
else
xml.writeBinaryProperty('sysex', this.sysex)
And the parser would automatically call the correct setter. If
getters and setters had to be paired, the getter would have to add
some sort of a marker saying whether the data is zipped, and then the
setter would have to split and switch based on that marker.
> > This is just the simplest solution I could think of without writing a
> > new parser.
>
> Well, I'm all for that. In fact, our two proposals don't seem to differ
> in regards to how the XML gets read, parsed, and sets the values in the
> objects. I think we only differ on how we generate the XML at save time, =
no?
True.
|
|
From: Joe E. <jo...@em...> - 2006-03-09 02:06:23
Attachments:
smime.p7s
|
Rib Rdb wrote:
> On 3/7/06, Joe Emenaker <jo...@em...> wrote:
>
>> Now, granted this is a problem that should only affect someone who's
>> authoring a subclass to a implementation of IPatch.... but the
>> combination of how easy it would be to forget to call super.write() and
>> the consequences for doing so... well... they worry me.
>>
>
> Any problem with serialization, no matter how it happens is going to
> cause problems until try to load it.
>
Well, not necessarily. Before we save any data element to XML, we can
make sure (at save time) that the proper method exists for putting that
data back into the object. Of course, this doesn't guard against saving
to XML, then removing the setter from the class, and then trying to load
the XML. But I don't think there's *any* way to solve situations like that.
>> My rough idea of the introspection would work something like this:
>> - Get a list of all public getters and setters for the object...
>>
>
> This has several problems. For one, the XML setters are not normal
> setters: they have to take a string argument.
Well, at the present time, all fields of a patch that we're saving
(except for the sysex itself) are strings. So, that's not a problem for
the *present* situation. Now, as for the future, you pointed out that...
> Someone who just writes
> a patch class without knowing about serialization is probably not
> going to write this type of setter.
True. Which is why I said "If any of those getters return anything other
than String (or any other data types we know how to save and restore),
then throw an exception, or fail, or complain, or do something". In
other words, if the author added a getter like "Hashtable
getHashtableofsomething()", then the XML saving code would throw up a
warning message saying "Hey! We don't know how to save
'java.util.Hashtable', so you're going to lose the data returned by
'getHashtableofsomething()'!".
> Also, you lose some flexibility. For example the write method could
> do something like this
> if (xml.useGzip())
> xml.writeGzippedProperty('gzippedSysex', this.sysex)
> else
> xml.writeBinaryProperty('sysex', this.sysex)
>
> And the parser would automatically call the correct setter. If
> getters and setters had to be paired, the getter would have to add
> some sort of a marker saying whether the data is zipped, and then the
> setter would have to split and switch based on that marker.
>
On the other hand, you could have all gzipping done in the XML code. If
a data element is gzipped, you add/change an attribute to the element.
<sysex> or <sysex compression="none"> versus <sysex compression="gzip">.
You can also set policies in the XML read/write code such as "gzip all
String objects longer than 200 bytes". This way, authors of future patch
classes don't have to brew their own compression/decompression stuff...
and existing patch classes don't have to have the compression code
retroactively added.
Now, I just thought of another way, which would use your "write()"
methods without requiring super.write(). Using inspection, I think it
*is* possible to make calls to the over-ridden methods of a superclass.
In other words, the XML writing code could call a class' write(), and
then call its superclass' write(), and then *that* superclass'
superclass' write(), etc.... until it got all the way to the original
class. But that's a little spooky.
- Joe
|
|
From: Joe E. <jo...@em...> - 2006-03-09 04:25:19
Attachments:
smime.p7s
|
Rib Rdb wrote: > On 3/8/06, Joe Emenaker <jo...@em...> wrote: > >> True. Which is why I said "If any of those getters return anything other >> than String (or any other data types we know how to save and restore), >> then throw an exception, or fail, or complain, or do something". In >> other words, if the author added a getter like "Hashtable >> getHashtableofsomething()", then the XML saving code would throw up a >> warning message saying "Hey! We don't know how to save >> 'java.util.Hashtable', so you're going to lose the data returned by >> 'getHashtableofsomething()'!". >> > > I still think this has problems. How do you handle transient data or > data which has multiple getters and setters? Well, I think I hinted, at one point, at the idea of, instead of trying to autodetect the public getters and setters, maintaining an array or vector of the property names to save. For example, if the vector contained "Name", "Author", and "Date", then the XML load and save code would only try to call getName, setName, getAuthor, setAuthor, etc. But, if my suggestions thus far have made you uncomfortable, then *this* idea has probably got you squirming. :) > Name has getters and > setters, but may also be stored inside sysex. Well, that's a device-specific trait, and it affects the current serialization method just as much as it would any new XML method. In other words, as JSL stands now, the "name" string is stored along with the sysex data and it's loaded with it, too. If individual driver classes choose to override getName and setName to access the sysex data directly and not use the "name" string, it shouldn't matter. > Sysex data can be > accessed as bytes or sysex messages, and now strings. How do we > decide when to throw exceptions for unstorable data and which data > isn't supposed to be stored? > Well, if we didn't have a vector of properties to save, then we'd have to use matching pairs of public getters/setters. >> Now, I just thought of another way, which would use your "write()" >> methods without requiring super.write(). Using inspection, I think it >> *is* possible to make calls to the over-ridden methods of a superclass. >> In other words, the XML writing code could call a class' write(), and >> then call its superclass' write(), and then *that* superclass' >> superclass' write(), etc.... until it got all the way to the original >> class. But that's a little spooky. >> > Yeah. And then you have problems if someone does call super.write(). > Well, I knew that when I suggested it.... but it's not that big of a problem because you'd just get two copies of some data items and, at worst, it would just result in duplicate calls to setName, setWhatever..... You're starting to convince me that the super.write() method is the path of least resistance... but I *have* tried this very solution before and it looked great on paper and turned into a bit of a pain when it was put into practice. So, forgive me if I'm a little gun-shy about it. - Joe |
|
From: Rib R. <ri...@gm...> - 2006-03-09 04:45:42
|
On 3/8/06, Joe Emenaker <jo...@em...> wrote: > Rib Rdb wrote: > > On 3/8/06, Joe Emenaker <jo...@em...> wrote: > > > >> True. Which is why I said "If any of those getters return anything oth= er > >> than String (or any other data types we know how to save and restore), > >> then throw an exception, or fail, or complain, or do something". In > >> other words, if the author added a getter like "Hashtable > >> getHashtableofsomething()", then the XML saving code would throw up a > >> warning message saying "Hey! We don't know how to save > >> 'java.util.Hashtable', so you're going to lose the data returned by > >> 'getHashtableofsomething()'!". > >> > > > > I still think this has problems. How do you handle transient data or > > data which has multiple getters and setters? > Well, I think I hinted, at one point, at the idea of, instead of trying > to autodetect the public getters and setters, maintaining an array or > vector of the property names to save. For example, if the vector > contained "Name", "Author", and "Date", then the XML load and save code > would only try to call getName, setName, getAuthor, setAuthor, etc. But, > if my suggestions thus far have made you uncomfortable, then *this* idea > has probably got you squirming. :) Hmmm. The more I think about it the more I like this idea. It's more uniform than having setters used for loading, but one method used for writing. We could add something like "List getXmlPropertyNames()" to IPatch. But then we still have the super problem. But as I said before, I really don't think new Patch classes are going to be made often, so we should just pick one. And calling super.write seems more like the java way to do it. Java doesn't have much magic. > > Name has getters and > > setters, but may also be stored inside sysex. > Well, that's a device-specific trait, and it affects the current > serialization method just as much as it would any new XML method. In > other words, as JSL stands now, the "name" string is stored along with > the sysex data and it's loaded with it, too. If individual driver > classes choose to override getName and setName to access the sysex data > directly and not use the "name" string, it shouldn't matter. > > Sysex data can be > > accessed as bytes or sysex messages, and now strings. How do we > > decide when to throw exceptions for unstorable data and which data > > isn't supposed to be stored? > > > Well, if we didn't have a vector of properties to save, then we'd have > to use matching pairs of public getters/setters. > >> Now, I just thought of another way, which would use your "write()" > >> methods without requiring super.write(). Using inspection, I think it > >> *is* possible to make calls to the over-ridden methods of a superclass= . > >> In other words, the XML writing code could call a class' write(), and > >> then call its superclass' write(), and then *that* superclass' > >> superclass' write(), etc.... until it got all the way to the original > >> class. But that's a little spooky. > >> > > Yeah. And then you have problems if someone does call super.write(). > > > Well, I knew that when I suggested it.... but it's not that big of a > problem because you'd just get two copies of some data items and, at > worst, it would just result in duplicate calls to setName, setWhatever...= .. > > You're starting to convince me that the super.write() method is the path > of least resistance... but I *have* tried this very solution before and > it looked great on paper and turned into a bit of a pain when it was put > into practice. So, forgive me if I'm a little gun-shy about it. > > - Joe |
|
From: Joe E. <jo...@em...> - 2006-03-09 12:07:54
Attachments:
smime.p7s
|
Rib Rdb wrote:
> On 3/8/06, Joe Emenaker <jo...@em...> wrote:
>
>> Rib Rdb wrote:
>>
>>> I still think this has problems. How do you handle transient data or
>>> data which has multiple getters and setters?
>>>
>> Well, I think I hinted, at one point, at the idea of, instead of trying
>> to autodetect the public getters and setters, maintaining an array or
>> vector of the property names to save. For example, if the vector
>> contained "Name", "Author", and "Date", then the XML load and save code
>> would only try to call getName, setName, getAuthor, setAuthor, etc. But,
>> if my suggestions thus far have made you uncomfortable, then *this* idea
>> has probably got you squirming. :)
>>
>
> Hmmm. The more I think about it the more I like this idea. It's more
> uniform than having setters used for loading, but one method used for
> writing. We could add something like "List getXmlPropertyNames()" to
> IPatch. But then we still have the super problem.
Well, yes and no. If the property names are kept in a vector or
something similar, then the vector would be created in the base class
and populated with some values via some add() method, like:
public Patch() {
addPropertyName("Author");
addPropertyName("Comment");
etc.
}
The default constructor is supposed to be called for all subclasses that
don't explicitly specify a super constructor in their own constructor.
So, if I subclass Patch in a class called SpecialPatch and write:
public SpecialPatch() {
addPropertyName("SomeSpecialData");
}
then the Author and Comment ones should still get put into the vector.
But now it's getting fairly cumbersome. We've got to have matching
getters and setters, they have to be public, and their name has to match
a String value in the PropertyName vector. So, at this point, I think it
would then be easier to just use a special data structure for all data
that we want to save (except the sysex. That's always special).
How about this. Patch would have some object of Name/Value pairs which
would contain all of the data that should be saved. This gets around all
of the getters/setters. In fact, a class wouldn't have to change their
code much at all. Things like:
patchName = getPatchNameFromSysexData();
would become:
setValue("PatchName",getPatchNameFromSysexData());
In support of this kind of method, I point out that the JAXP DOM stuff
already has something like this in NamedNodeMap.
The three problems I see with this are:
1 - The simple implementation I've proposed here only deals with
strings. But there's nothing to keep us from making setIntValue(String
name, int val), and setBoolVal(..) etc.
2 - There's nothing to purge old deprecated data. If Patch has a field
called "Author" and then we decide, later, that we don't want an
"Author" field anymore and strip any mention of it out of Patch, the
Author data items already existing in XML files will continue to get
loaded and saved.... even though they're never used by JSL. It's a weird
version of a memory leak, if you will.
3 - Name collision. There's nothing to keep subclasses from stomping on
values in the superclass. If you subclassed Patch, you'd need to know
that Patch already uses "Author" so that you didn't try to use it
yourself for a different purpose. Even worse, if we went in and added a
property name to the Patch superclass, it could collide with a property
name in one of Patch's subclasses.
> But as I said
> before, I really don't think new Patch classes are going to be made
> often, so we should just pick one. And calling super.write seems more
> like the java way to do it. Java doesn't have much magic.
>
Well, if you don't feel like we're working our way any closer to a
better solution, then we can just go with super.write(). The one thing
I'd want to change, however, is that I think we should have a read()
method as well. Here's why...
The current "super.write()" proposal, as I understand it, is to have
write() do things like:
xml.writeProperty("author", self.author);
and then we need a setAuthor(String) method. I'm still wincing over the requirement that "author" in the call to writeProperty match "setAuthor". However, if we had a read() method that did things like:
self.author = xml.readProperty("author");
*then* we can put "author" in a constant, so we'd end up with code like this:
private static final String AuthorXMLName = "author";
xml.writeProperty(AuthorXMLName, self.author);
xml.readProperty(AuthorXMLName);
which is what you can see I did in my XMLFileUtils class. It makes it *much* more likely that your IDE will catch any typos. Also, reading is done just like writing. Lastly, the readProperty() method(s) should be really easy to write, since we can have the "xml." part of "xml.readProperty" holding the DOM Node for just that patch... and readProperty could just turn right around return the value returned by "getAttribute()" for that node.
- Joe
|
|
From: Rib R. <ri...@gm...> - 2006-03-09 19:25:54
|
On 3/9/06, Joe Emenaker <jo...@em...> wrote:
> Rib Rdb wrote:
> > On 3/8/06, Joe Emenaker <jo...@em...> wrote:
> >
> >> Rib Rdb wrote:
> >>
> >>> I still think this has problems. How do you handle transient data or
> >>> data which has multiple getters and setters?
> >>>
> >> Well, I think I hinted, at one point, at the idea of, instead of tryin=
g
> >> to autodetect the public getters and setters, maintaining an array or
> >> vector of the property names to save. For example, if the vector
> >> contained "Name", "Author", and "Date", then the XML load and save cod=
e
> >> would only try to call getName, setName, getAuthor, setAuthor, etc. Bu=
t,
> >> if my suggestions thus far have made you uncomfortable, then *this* id=
ea
> >> has probably got you squirming. :)
> >>
> >
> > Hmmm. The more I think about it the more I like this idea. It's more
> > uniform than having setters used for loading, but one method used for
> > writing. We could add something like "List getXmlPropertyNames()" to
> > IPatch. But then we still have the super problem.
> Well, yes and no. If the property names are kept in a vector or
> something similar, then the vector would be created in the base class
> and populated with some values via some add() method, like:
> public Patch() {
> addPropertyName("Author");
> addPropertyName("Comment");
> etc.
> }
That would work, but that's probably something that should go in an
implementation not the actual interface.
>
> The default constructor is supposed to be called for all subclasses that
> don't explicitly specify a super constructor in their own constructor.
> So, if I subclass Patch in a class called SpecialPatch and write:
> public SpecialPatch() {
> addPropertyName("SomeSpecialData");
> }
>
> then the Author and Comment ones should still get put into the vector.
Do you really think anyone is ever going to subclass Patch. Have you
seen how much of it is final? The main reason IPatch was created was
so people don't have to subclass Patch. I think the main thing we
need to worry about is new implementations of IPatch (which are still
going to be rare and only done by people with an advanced knowledge of
JSynthLib), not subclasses of the existing two implementations.
> But now it's getting fairly cumbersome. We've got to have matching
> getters and setters, they have to be public, and their name has to match
> a String value in the PropertyName vector. So, at this point, I think it
> would then be easier to just use a special data structure for all data
> that we want to save (except the sysex. That's always special).
>
> How about this. Patch would have some object of Name/Value pairs which
> would contain all of the data that should be saved. This gets around all
> of the getters/setters. In fact, a class wouldn't have to change their
> code much at all. Things like:
> patchName =3D getPatchNameFromSysexData();
> would become:
> setValue("PatchName",getPatchNameFromSysexData());
>
> In support of this kind of method, I point out that the JAXP DOM stuff
> already has something like this in NamedNodeMap.
>
> The three problems I see with this are:
> 1 - The simple implementation I've proposed here only deals with
> strings. But there's nothing to keep us from making setIntValue(String
> name, int val), and setBoolVal(..) etc.
> 2 - There's nothing to purge old deprecated data. If Patch has a field
> called "Author" and then we decide, later, that we don't want an
> "Author" field anymore and strip any mention of it out of Patch, the
> Author data items already existing in XML files will continue to get
> loaded and saved.... even though they're never used by JSL. It's a weird
> version of a memory leak, if you will.
> 3 - Name collision. There's nothing to keep subclasses from stomping on
> values in the superclass. If you subclassed Patch, you'd need to know
> that Patch already uses "Author" so that you didn't try to use it
> yourself for a different purpose. Even worse, if we went in and added a
> property name to the Patch superclass, it could collide with a property
> name in one of Patch's subclasses.
I really don't like this idea. And how do you enforce it with an
interface? You would have to add getter and setter methods for this
data structure. Read and write methods seems a lot simpler.
> > But as I said
> > before, I really don't think new Patch classes are going to be made
> > often, so we should just pick one. And calling super.write seems more
> > like the java way to do it. Java doesn't have much magic.
> >
> Well, if you don't feel like we're working our way any closer to a
> better solution, then we can just go with super.write().
Actually I meant super.getXmlPropertyNames.
> The one thing I'd want to change, however, is that I think we should have=
a read()
> method as well. Here's why...
>
> The current "super.write()" proposal, as I understand it, is to have
> write() do things like:
>
> xml.writeProperty("author", self.author);
>
> and then we need a setAuthor(String) method. I'm still wincing over the r=
equirement that "author" in the call to writeProperty match "setAuthor". Ho=
wever, if we had a read() method that did things like:
>
> self.author =3D xml.readProperty("author");
>
> *then* we can put "author" in a constant, so we'd end up with code like t=
his:
>
> private static final String AuthorXMLName =3D "author";
>
> xml.writeProperty(AuthorXMLName, self.author);
>
> xml.readProperty(AuthorXMLName);
>
> which is what you can see I did in my XMLFileUtils class. It makes it *mu=
ch* more likely that your IDE will catch any typos. Also, reading is done j=
ust like writing.
I certainly like the idea of having a read method. But the parser I
wrote is event based (SAX), so a read method doesn't easily translate.
But I suppose we could make it work.
> Lastly, the readProperty() method(s) should be really easy to write, sinc=
e we can have the "xml." part of "xml.readProperty" holding the DOM Node fo=
r just that patch... and readProperty could just turn right around return t=
he value returned by "getAttribute()" for that node.
Do you mean to store all the data as attributes of an element instead
of children? That sounds like a bad idea, and my parser certainly
won't be able to handle that.
|
|
From: Joe E. <jo...@em...> - 2006-03-09 23:01:38
Attachments:
smime.p7s
|
Rib Rdb wrote:
> On 3/9/06, Joe Emenaker <jo...@em...> wrote:
>
>> The default constructor is supposed to be called for all subclasses that
>> don't explicitly specify a super constructor in their own constructor.
>> So, if I subclass Patch in a class called SpecialPatch and write:
>> public SpecialPatch() {
>> addPropertyName("SomeSpecialData");
>> }
>>
>> then the Author and Comment ones should still get put into the vector.
>>
>
> Do you really think anyone is ever going to subclass Patch.
I don't understand why Scene didn't, actually.
> Have you seen how much of it is final?
Well, that part's easy. You just delete the "final" part. :)
> The main reason IPatch was created was
> so people don't have to subclass Patch.
"so people don't have to" make use of the existing code in Patch and,
instead, have to write every method anew? Well, we shouldn't dwell on
"how come it wasn't done *this* way?" stuff.
> I certainly like the idea of having a read method. But the parser I
> wrote is event based (SAX), so a read method doesn't easily translate.
> But I suppose we could make it work.
>
Well, it doesn't appear to me that our data format is going to require
SAX and the "order-ness" that it provides. I mean, if the <Author>
element comes before or after the <Sysex> element... what does it
matter, as long as they all appear within the same <Patch> element?
Furthermore, does SAX even have an easy system for writing the XML to a
file? With DOM, you can read an XML into a Document and then save it
write back out with a couple of lines of code.
>> Lastly, the readProperty() method(s) should be really easy to write, since we can have the "xml." part of "xml.readProperty" holding the DOM Node for just that patch... and readProperty could just turn right around return the value returned by "getAttribute()" for that node.
>>
> Do you mean to store all the data as attributes of an element instead
> of children?
Not necessarily. In fact, I was going to touch on that issue next.
Although all of the data that's currently saved (except for sysex) is
short enough for use as attributes, we wouldn't have control over
subclasses (and/or future implementations of IPatch). So, we could
either give the author a choice (like xml.writeElement() and
xml.writeAttribute()) or just force everything to be an Element.
I'm leaning toward just making everything an Element. Partly because you
see to really want that and also because my next suggestion is that we
make this whole thing not XML-specific. In other words, instead of a
void write(XMLWriter xml)
method, it could be
void write(PropertyHolder props)
and PropertyHolder (or... "PropertyBasket" if we want to use exiting JSL
nomenclature :) ) would just hold all of the values that the patch
object wanted to be saved. So, when a user clicks "Save" in JSL, JSL
would...
1 - Create an empty DOM
2 - Get the next patch in the PatchBasket
3 - Create an empty "patch" DOM Element
4 - Create an empty PropertyHolder
5 - Call the patch's write(PropertyHolder) method with the new, empty
one we just made
6 - Write those properties to the patch DOM Element
7 - Back to step 2
The things to note here are that the patch classes would have no idea
that we were using XML at all. Only the methods doing the actual
save/load would know. This would allow us to change the format, or to
support various formats. Even if we never decided to change the *file*
format again, we could still use the PropertyHolder idea to support
other formats for other purposes.... like transmission over the net or
copy/paste or something like that.
- Joe
|
|
From: Rib R. <ri...@gm...> - 2006-03-10 02:18:11
|
On 3/9/06, Joe Emenaker <jo...@em...> wrote:
> Rib Rdb wrote:
> > On 3/9/06, Joe Emenaker <jo...@em...> wrote:
> >
> >> The default constructor is supposed to be called for all subclasses th=
at
> >> don't explicitly specify a super constructor in their own constructor.
> >> So, if I subclass Patch in a class called SpecialPatch and write:
> >> public SpecialPatch() {
> >> addPropertyName("SomeSpecialData");
> >> }
> >>
> >> then the Author and Comment ones should still get put into the vector.
> >>
> >
> > Do you really think anyone is ever going to subclass Patch.
> I don't understand why Scene didn't, actually.
I don't know why it didn't originally, but if it used a subclass of
Patch then you couldn't have a scene containing XMLPatches
> > Have you seen how much of it is final?
> Well, that part's easy. You just delete the "final" part. :)
I would assume we put those there for a reason.
> > The main reason IPatch was created was
> > so people don't have to subclass Patch.
> "so people don't have to" make use of the existing code in Patch and,
> instead, have to write every method anew? Well, we shouldn't dwell on
> "how come it wasn't done *this* way?" stuff.
"so people don't have to" be restricted by the design of the Patch class.
> > I certainly like the idea of having a read method. But the parser I
> > wrote is event based (SAX), so a read method doesn't easily translate.
> > But I suppose we could make it work.
> >
> Well, it doesn't appear to me that our data format is going to require
> SAX and the "order-ness" that it provides. I mean, if the <Author>
> element comes before or after the <Sysex> element... what does it
> matter, as long as they all appear within the same <Patch> element?
> Furthermore, does SAX even have an easy system for writing the XML to a
> file? With DOM, you can read an XML into a Document and then save it
> write back out with a couple of lines of code.
I'm not saying we need sax. I'm just trying to explain why I
suggested using setters instead of a single read method -- There's
already a parser in CVS that works that way.
> >> Lastly, the readProperty() method(s) should be really easy to write, s=
ince we can have the "xml." part of "xml.readProperty" holding the DOM Node=
for just that patch... and readProperty could just turn right around retur=
n the value returned by "getAttribute()" for that node.
> >>
> > Do you mean to store all the data as attributes of an element instead
> > of children?
> Not necessarily. In fact, I was going to touch on that issue next.
> Although all of the data that's currently saved (except for sysex) is
> short enough for use as attributes, we wouldn't have control over
> subclasses (and/or future implementations of IPatch). So, we could
> either give the author a choice (like xml.writeElement() and
> xml.writeAttribute()) or just force everything to be an Element.
>
> I'm leaning toward just making everything an Element. Partly because you
> see to really want that and also because my next suggestion is that we
> make this whole thing not XML-specific. In other words, instead of a
>
> void write(XMLWriter xml)
>
> method, it could be
>
> void write(PropertyHolder props)
>
> and PropertyHolder (or... "PropertyBasket" if we want to use exiting JSL
> nomenclature :) ) would just hold all of the values that the patch
> object wanted to be saved. So, when a user clicks "Save" in JSL, JSL
> would...
>
> 1 - Create an empty DOM
> 2 - Get the next patch in the PatchBasket
> 3 - Create an empty "patch" DOM Element
> 4 - Create an empty PropertyHolder
> 5 - Call the patch's write(PropertyHolder) method with the new, empty
> one we just made
> 6 - Write those properties to the patch DOM Element
> 7 - Back to step 2
>
> The things to note here are that the patch classes would have no idea
> that we were using XML at all. Only the methods doing the actual
> save/load would know. This would allow us to change the format, or to
> support various formats. Even if we never decided to change the *file*
> format again, we could still use the PropertyHolder idea to support
> other formats for other purposes.... like transmission over the net or
> copy/paste or something like that.
I guess I just prefer the extreme programming approach. Why make it
more complicated now to support something that we might possibly want
to use at some point in the future. I'd rather keep the design simple
now, and then when it needs to change, find the best design for the
new requirements. But whatever. I don't have time to write a new
parser and XML writer. If you do, go right ahead.
|
|
From: Joe E. <jo...@em...> - 2006-03-10 03:06:14
Attachments:
smime.p7s
|
Rib Rdb wrote: > On 3/9/06, Joe Emenaker <jo...@em...> wrote: > >> Rib Rdb wrote: >> >>> The main reason IPatch was created was >>> so people don't have to subclass Patch. >>> >> "so people don't have to" make use of the existing code in Patch and, >> instead, have to write every method anew? Well, we shouldn't dwell on >> "how come it wasn't done *this* way?" stuff. >> > > "so people don't have to" be restricted by the design of the Patch class. > Well, I think Patch should have been a fairly minimal abstract class, and then SinglePatch and BankPatch would be subclasses of those with the Single- and Bank-specific methods... but I wasn't around when those decisions were made, and what's done is done. I'm just saying that there was a way to do it that didn't require every new type of patch to have to redefine all of the methods *and* which wouldn't overly restrict those new types of patches. >> This would allow us to change the format, or to >> support various formats. Even if we never decided to change the *file* >> format again, we could still use the PropertyHolder idea to support >> other formats for other purposes.... like transmission over the net or >> copy/paste or something like that. >> > > I guess I just prefer the extreme programming approach. Why make it > more complicated now to support something that we might possibly want > to use at some point in the future. I'd rather keep the design simple > now, and then when it needs to change, find the best design for the > new requirements. Well, that's how we got what we have now. Putting everything in core.* was a simple design that worked... until things started getting big. The current midi layer was a simple design that worked... until multiple things (like the midi monitor) needed to be notified of the same sysex messages. Now we have a half-dozen logging methods in MidiUtil and Actions all feeding into MidiMonitor, and they're getting called from a half-dozen other methods in PatchEdit and MidiUtil. Temporary fixes turn into permanent ones. Simple "just for now" solutions get appended to until they're unruly and unmanageable. So, the best way to save yourself grief down the road is to make sure that you design something now that isn't too complicated to use today, yet flexible to accommodate all the stuff you can imagine needing in the future. > But whatever. I don't have time to write a new > parser and XML writer. If you do, go right ahead. > I have time to write the parser and the XML writer.... and I can write the PropertyHolder or whatever we want to call it. All you'd need to do is support a read(PropertyHolder) and write(PropertyHolder) for any given patch in your XMLDriver code. - Joe |
|
From: Rib R. <ri...@gm...> - 2006-03-10 07:34:08
|
On 3/9/06, Joe Emenaker <jo...@em...> wrote: > Rib Rdb wrote: > > On 3/9/06, Joe Emenaker <jo...@em...> wrote: > > > >> Rib Rdb wrote: > >> > >>> The main reason IPatch was created was > >>> so people don't have to subclass Patch. > >>> > >> "so people don't have to" make use of the existing code in Patch and, > >> instead, have to write every method anew? Well, we shouldn't dwell on > >> "how come it wasn't done *this* way?" stuff. > >> > > > > "so people don't have to" be restricted by the design of the Patch clas= s. > > > Well, I think Patch should have been a fairly minimal abstract class, > and then SinglePatch and BankPatch would be subclasses of those with the > Single- and Bank-specific methods... but I wasn't around when those > decisions were made, and what's done is done. I'm just saying that there > was a way to do it that didn't require every new type of patch to have > to redefine all of the methods *and* which wouldn't overly restrict > those new types of patches. > >> This would allow us to change the format, or to > >> support various formats. Even if we never decided to change the *file* > >> format again, we could still use the PropertyHolder idea to support > >> other formats for other purposes.... like transmission over the net or > >> copy/paste or something like that. > >> > > > > I guess I just prefer the extreme programming approach. Why make it > > more complicated now to support something that we might possibly want > > to use at some point in the future. I'd rather keep the design simple > > now, and then when it needs to change, find the best design for the > > new requirements. > Well, that's how we got what we have now. Putting everything in core.* > was a simple design that worked... until things started getting big. The > current midi layer was a simple design that worked... until multiple > things (like the midi monitor) needed to be notified of the same sysex > messages. Now we have a half-dozen logging methods in MidiUtil and > Actions all feeding into MidiMonitor, and they're getting called from a > half-dozen other methods in PatchEdit and MidiUtil. > > Temporary fixes turn into permanent ones. Simple "just for now" > solutions get appended to until they're unruly and unmanageable. So, the > best way to save yourself grief down the road is to make sure that you > design something now that isn't too complicated to use today, yet > flexible to accommodate all the stuff you can imagine needing in the futu= re. > > But whatever. I don't have time to write a new > > parser and XML writer. If you do, go right ahead. > > > I have time to write the parser and the XML writer.... and I can write > the PropertyHolder or whatever we want to call it. All you'd need to do > is support a read(PropertyHolder) and write(PropertyHolder) for any > given patch in your XMLDriver code. I shouild be able to do that. |
|
From: Joe E. <jo...@em...> - 2006-03-10 10:35:09
Attachments:
smime.p7s
|
Rib Rdb wrote: > On 3/9/06, Joe Emenaker <jo...@em...> wrote: > >> I have time to write the parser and the XML writer.... and I can write >> the PropertyHolder or whatever we want to call it. All you'd need to do >> is support a read(PropertyHolder) and write(PropertyHolder) for any >> given patch in your XMLDriver code. >> > I shouild be able to do that. > So, to clarify then.... any implementation of IPatch would need to allow setting of the sysex data and then it should implement read(PropertyHolder) and write(PropertyHolder). Well, read and write are a little vague. How about something like importProperties and exportProperties? I would have suggested getProperties/setProperties, but getters usually return something and, in this case, we're going to want to give the class a PropertyHolder and then have it populate it. Also, unless you like PropertyHolder, I'd prefer something a little different. Maybe PropertySet or ValueSet. Or maybe you can suggest something that you think sounds like what it does? - Joe |
|
From: Rib R. <ri...@gm...> - 2006-03-10 17:34:41
|
On 3/10/06, Joe Emenaker <jo...@em...> wrote: > Rib Rdb wrote: > > On 3/9/06, Joe Emenaker <jo...@em...> wrote: > > > >> I have time to write the parser and the XML writer.... and I can write > >> the PropertyHolder or whatever we want to call it. All you'd need to d= o > >> is support a read(PropertyHolder) and write(PropertyHolder) for any > >> given patch in your XMLDriver code. > >> > > I shouild be able to do that. > > > So, to clarify then.... any implementation of IPatch would need to allow > setting of the sysex data and then it should implement > read(PropertyHolder) and write(PropertyHolder). What do you mean "allow setting of the sysex data" > Well, read and write are > a little vague. How about something like importProperties and > exportProperties? I would have suggested getProperties/setProperties, > but getters usually return something and, in this case, we're going to > want to give the class a PropertyHolder and then have it populate it. > > Also, unless you like PropertyHolder, I'd prefer something a little > different. Maybe PropertySet or ValueSet. Or maybe you can suggest > something that you think sounds like what it does? How about importProperties( PropertyMap props )? |
|
From: Joe E. <jo...@em...> - 2006-03-11 11:46:15
Attachments:
smime.p7s
|
Rib Rdb wrote: > What do you mean "allow setting of the sysex data" > Well, we kinda need some way for the actual sysex data to be put into (and retrieved from) a patch object, no? Right now, I think it's done in the actual implementations (for example, Patch.Patch(byte[] sysex) ), but it's not required by the interface itself. To move forward, we either need a way to get/set the sysex of a patch *or* we need a way for IPatch implementations to put the patch data in the set of properties they return. >> Also, unless you like PropertyHolder, I'd prefer something a little >> different. Maybe PropertySet or ValueSet. Or maybe you can suggest >> something that you think sounds like what it does? >> > How about importProperties( PropertyMap props )? > One thing that dawned on me today was "We probably shouldn't use the word 'Properties', because Java already has a standard class called that, and this would confuse people because they'd think that our "Properties" had something to do with the standard Java "Properties". However, just now, I realized... we *could* just use the plain Java Properties class, since it just is a String-to-String mapping. The thought I had was that I could extend Properties to something called, say, PatchProperties which would have the following additional functionality: 1 - An exception or ErrMsg would be generated any time an existing property name was set (to catch cases where subclasses accidentally used a property name that was used by the superclass) 2 - There'd be a set of convenience methods like getInt(String propertyName), getBoolean(String propertyName), setInt(String propertyName, int value), etc. In this case, importProperties( PatchProperties props) and exportProperties( PatchProperties props) would be entirely appropriate. - Joe |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X