On 18-Jan-09, at 12:35 PM, Sean M. Pappalardo wrote: > Hello. > > Albert Santoni wrote: >> // Construct row in table >> addRow(device, group, key, controltype, miditype, midino, >> midichan, option); >> >> You basically need to pass the information in those parameters back >> from >> MidiMapping somehow. Perhaps that information should be contained >> in a > > Exactly, so I made a QHash (since all those parameters are QStrings > anyway) keying on the parameter names like so: > > QHash<QString, QString> parameter; > parameter["device"]=device; > parameter["midino"]=midino; > ... > > ...and so on for each parameter. Then I just pile each QHash (for each > addRow call) onto a QList. Then dlgprefmidibindings calls > getParameterList() which returns a reference to that QList, which > dlgprefmidibinding can use to then pull off each QHash in turn and > call > addRow(parameter["device"], parameter["midino"], etc.) until the > list is > empty. > > While that may not be the best way to do it, it's compact for > transmission between objects and QHashes are supposed to be pretty > efficient. I also made sure to delete the QLists when I'm done with > them > (via MidiMapping::deleteRowParams() which gets called from > dlgprefmidibindings after it's done with the QLists.) > > Sound reasonable?
This feels like an awkward solution to me (IMO). When I first read this I thought making a struct with those variables and passing that around would be better, but I think that'd be just be another half- solution to the real problem. I think the real problem is a conceptual one - Each "mapping" of a <device, controltype, miditype, midino, midichan> onto a <group, key> should be an object, and it currently isn't. The MidiMapping class you're creating should contain a list of these "mappings", and perhaps the name of your class should be changed then too (to something more plural). One advantage of abstracting things like this is that it simplifies the XML serialization logic. Consider for example, your MidiMapping class. The load() function would create the list of mappings by creating a new object for each <control> element, and passing that QDomElement block into the constructor of that class. The save() function in MidiMapping would iterate through the list of mappings and call a separate save() member function (belonging to their class) on each of the objects. I'm going to stop writing now because this isn't going to make any sense when you read it. Maybe we can discuss this on IRC sometime. The general idea is that the structure of the XML should correspond to the class structure of your application, which makes serialization very elegant. > >> your codebase with XML parsing crap everywhere. Instead of going >> structFoobar->midi_note, you have to walk through this XML tree. > > That's probably more elegant that a QHash, but it's only needed one > time > on XML loading and can be deleted after that. Yeah, what I just wrote above makes sense if you don't want to use the XML as a data structure at runtime. I think the tradeoff is very much in the favour of having each class serialize itself, etc. > > > ----- > > All that said, I'm trying to compile the monster and getting a > series of > errors I can't figure out. (And I've learned from past experience > that I > need to fix the error at the top of the list before worrying about any > others since they can all change.) > > So I'm getting this: > src/script/midiscriptengine.h:47: error: 'MidiCategory' has not been > declared #include "configobject.h" in midiscriptengine.h. There's something weird about MidiCategory and I couldn't get a forward declaration for it to work when I was doing my MIDI stuff. RANT: And as a note for anyone who's reading this - "Midi Category" is a meaningless term. Lesson: When you're going to write code that implements some specification, stick to the terminology used in the specification. The MIDI code we have is very difficult to read because it doesn't use the terminology defined in the MIDI spec (and commonly used everywhere else). Thanks, Albert ------------------------------------------------------------------------------ This SF.net email is sponsored by: SourcForge Community SourceForge wants to tell your story. http://p.sf.net/sfu/sf-spreadtheword _______________________________________________ Mixxx-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/mixxx-devel
