ClarionMag reader Greg Fasolt recently contacted me with a question about a class I wrote some years ago, and which can be found at http://archive.clarionmag.com/cmag/v4/v4n06checkbox2.html. That class makes it easy (or at least easier) to manage many to many links between two browses by way of checkboxes, like this:
The window shown links a list of students with a list of courses (based on the School sample app).
There are two ABC browses here - the one on the left is page loaded (but could be file loaded) and lists all students; the one on the right is file loaded and lists all courses. The many-to-many checkbox class automatically creates or removes linking records whenever you click on a list box.
Greg was having some issues trying to use the class with a list box on a form, and he was also interested in some functionality the class didn't support.
Ye olde code
Revisiting code is almost always an instructional, if not always pleasant, experience.
I loaded up the sample app in C9, but when I generated and compiled I got a whack of compile errors caused by Clarion's failure to find the class...
... and quite a few more like that.
I looked for a global include pointing to the class header file, and didn't find one. So I opened up the file in question, which has the highly informative name "ccibrowb.inc".
There's nothing like an 8.3 filename to bring a nostalgic tear to my eye. Either that or looking at old code is like cutting onions. The class definition clearly violates two of my current coding rules: class names should be clearly descriptive, and the file name should be the same as the class name. I'll come back to that (eventually).
At the top of ccibrowb.inc is a line that identifies this file as ABC compatible, which means I'm expecting the IDE to scan the file and include the class in the application's internal list of available ABC classes:
!ABCIncludeFile
It's a long time since I created an ABC-compatible class, and I'm not a big fan of the ABC compatibility mechanism. I think it's fine for SoftVelocity's own classes; I just don't like writing my code this way anymore if I can help it. So seeing this line of text put me back on my heels for a bit.
I might or might not want to keep this code as ABC-compatible, but not being able to compile was a showstopper.
Loading custom ABC classes
The Clarion IDE only scans certain directories for ABC-compatible classes, which is why it couldn't find my class; these settings are under Tools | Options | Clarion | Clarion for Windows | Versions.
I could put my class files in the Accessory\libsrc\win directory, but I think some major changes may be in the offing, and this class may or may not be ABC when I'm done. So I wiped my watery eyes, held my nose and added my current application directory to the list.
Actually because the directory dialog that opens is looking for a file, I had to select a file in that directory before I could add to the list. The file I selected was ignored.
With that done I tried generating and compiling and got the same errors as before. Why? Because the IDE hadn't refreshed its list of classes. This can be forced from Global Properties | Actions, but from old habit I closed and opened the app and that did the job too.
I ran the app and verified that everything worked.
Source code
Here's the class header, in all its glory:
!ABCIncludeFile OMIT('_EndOfInclude_',_cciBrowseBPresent_) _cciBrowseBPresent_ EQUATE(1) include('abbrowse.inc') cciBrowseClassBParams group,type LinkFM &FileManager LinkKey &Key LinkLeftField &long LinkLeftFieldName string(255) LinkRightField &long LeftPrimaryID &long RightPrimaryID &long ViewQ &queue ViewQIconField any ViewQRightField &long end cciBrowseClassB CLASS(BrowseClass),TYPE,MODULE('ccibrowb.clw'),| LINK('ccibrowb.clw',_ABCLinkMode_),DLL(_ABCDllMode_) LastTime long,protected DebugQ &queue Debug byte(0) DataQ &DataQ,protected RightPrimaryID &long,protected LinkLeftField &long,protected LinkLeftFieldName cstring(255),protected LinkRightField &long,protected LinkKey &key LeftPrimaryID &long,protected lc long,protected LinkFM &FileManager,protected ViewQ &queue,protected ViewQIconField any ViewQRightField &long !--- methods --- DebugMsg procedure(String msg) Init procedure(FileManager LinkFM,Key LinkKey,*long LinkLeftField,| *long LinkRightField,*long LeftPrimaryID,*long RightPrimaryID,| Queue ViewQ,*? ViewQIconField,*long ViewQRightField) Init procedure(cciBrowseClassBParams params) Kill procedure,virtual LoadCheckboxData procedure RedisplayRecord procedure SaveCheckboxData procedure SetIcons procedure TakeEvent PROCEDURE,virtual end _EndOfInclude_ _EndOfInclude_
And the class source:
MEMBER omit('***',_c55_) _ABCDllMode_ EQUATE(0) _ABCLinkMode_ EQUATE(1) *** MAP . INCLUDE('ccibrowb.inc') include('keycodes.clw') dataQ queue,type ID long CurrValue long Changed byte end cciBrowseClassB.RedisplayRecord PROCEDURE() ChangeIt byte(ChangeRecord) Finished byte(RequestCompleted) code self.ResetFromAsk(ChangeIt,Finished) cciBrowseClassB.Init procedure(cciBrowseClassBparams paramsB) code self.Init( | paramsB.LinkFM, | paramsB.LinkKey, | paramsB.LinkLeftField, | ! paramsB.LinkLeftFieldName | paramsB.LinkRightField, | paramsB.LeftPrimaryID, | paramsB.RightPrimaryID, | paramsB.ViewQ, | paramsB.ViewQIconField, | paramsB.ViewQRightField) !string LinkLeftFieldName, cciBrowseClassB.Init procedure(FileManager LinkFM,Key LinkKey,| *long LinkLeftField,*long LinkRightField,| *long LeftPrimaryID,*long RightPrimaryID,| Queue ViewQ,*? ViewQIconField,*long ViewQRightField) ListControl long !FileManager LinkFM ! File manager for linking file !Key LinkKey ! Primary key for linking file !*long LinkLeftField ! Linking file left field !*long LinkRightField ! Linking file right field !*long LeftPrimaryID ! Left file primary key field !*long RightPrimaryID ! Right file primary key field !any ViewQIconField ! local for checkbox display code self.Debug = true self.FileLoaded = 1 self.LinkFM &= LinkFM self.LinkKey &= LinkKey self.RightPrimaryID &= RightPrimaryID self.LeftPrimaryID &= LeftPrimaryID self.LinkLeftField &= LinkLeftField !self.LinkLeftFieldName = clip(LinkLeftFieldName) self.LinkRightField &= LinkRightField self.ViewQRightField &= ViewQRightField self.DataQ &= new DataQ self.ViewQIconField &= ViewQIconField self.ViewQ &= ViewQ !self.LeftValue = 0 self.RetainRow = 1 compile('***',_c55_) self.lc = self.ilc.getControl() *** omit('***',_c55_) self.lc = self.ListControl *** cciBrowseClassB.Kill procedure code self.ViewQIconField &= null free(self.DataQ) dispose(self.DataQ) parent.kill() cciBrowseClassB.DebugMsg procedure(String msg) code if self.Debug and ~(self.DebugQ &= null) self.DebugQ = msg add(self.DebugQ) end cciBrowseClassB.LoadCheckboxData procedure x long code self.DebugMsg('called LoadCheckboxData with leftPrimaryID ' & self.LeftPrimaryID) free(self.DataQ) self.LinkRightField = 0 self.LinkLeftField = self.LeftPrimaryID set(self.LinkKey,self.LinkKey) loop while self.LinkFM.next() = level:benign if self.LinkLeftField <> self.LeftPrimaryID break end self.DataQ.ID = self.LinkRightField self.DataQ.CurrValue = 1 self.DataQ.Changed = false add(self.DataQ,self.DataQ.ID) self.DebugMsg('Added dataq record for LinkRightField ' & self.LinkRightField) end self.DebugMsg(records(self.dataq) & ' records loaded by LoadCheckboxData') self.SetIcons() cciBrowseClassB.SaveCheckboxData procedure select cstring(500) x long code self.DebugMsg('called SaveCheckboxData') loop x = 1 to records(self.DataQ) get(self.DataQ,x) if (self.DataQ.Changed) self.debugmsg('element ' & x & ' has changed, value is now ' & self.dataq.currvalue) if self.DataQ.CurrValue = 0 self.debugmsg('DELETING') ! If the link exists, remove it self.LinkLeftField = self.LeftPrimaryID self.LinkRightField = self.RightPrimaryID !self.DebugMsg(self.LinkLeftField & '/' & self.LinkRightField & '') if self.LinkFM.Fetch(self.LinkKey) = level:benign self.debugmsg('item found, calling deleterecord') !self.DebugMsg('** deleted *** (' & self.ViewQIconField & '/' & self.RightPrimaryID & ')') compile('***',_c55_) self.linkFM.DeleteRecord(0) *** omit('***',_c55_) delete(self.LinkFM.File) *** end else self.debugmsg('INSERTING') ! Create the link self.LinkLeftField = self.LeftPrimaryID self.LinkRightField = self.RightPrimaryID self.LinkFM.TryInsert() self.DebugMsg('** added *** (' & self.ViewQIconField & '/' & self.RightPrimaryID & ')') end self.DataQ.Changed = false put(self.DataQ) else self.debugmsg('element ' & x & ' has NOT changed, value is ' & self.dataq.currvalue) end end cciBrowseClassB.SetIcons PROCEDURE() x long code self.DebugMsg('SetIcons (' & records(self.viewq) & ')') loop x = 1 to records(self.ViewQ) !self.DebugMsg('Getting viewq record ' & x) get(self.ViewQ,x) self.DataQ.ID = self.ViewQRightField self.DebugMsg('got self.dataq.id ' & self.dataq.id) get(self.DataQ,self.DataQ.ID) if errorcode() !self.DebugMsg('SetIcons did not find record in DataQ for primary ' & self.ViewQRightField & ', set ViewQIconField to 1') self.ViewQIconField = 1 else self.ViewQIconField = choose(self.DataQ.CurrValue=1,2,1) self.DebugMsg('SetIcons found record in DataQ for primary ' & self.ViewQRightField | & ', set ViewQIconField to ' & self.ViewQIconField) end put(self.ViewQ) end cciBrowseClassB.TakeEvent PROCEDURE code parent.TakeEvent() self.DebugMsg('field ' & field() & ', event ' & event() & ', keycode ' & keycode()) if field() = self.lc if event() = event:Accepted | and keycode() = MouseLeft | and self.lc{proplist:mouseuprow} = self.lc{proplist:mousedownrow} | and self.lc{proplist:mouseupfield} = self.lc{proplist:mousedownfield} | and self.lc{proplist:mousedownfield} = 1 self.debugmsg('clicked on checkbox') ! ! Get the current record ! self.UpdateViewRecord() ! ! Update the buffer ! self.UpdateBuffer() get(self.ViewQ,choice(self.lc)) self.DataQ.ID = self.ViewQRightField get(self.DataQ,self.DataQ.ID) if errorcode() !self.debugmsg('no dataq record for id ' & self.ViewQRightField & ', adding now') self.DataQ.ID = self.RightPrimaryID self.DataQ.CurrValue = 1 self.DataQ.Changed = true add(self.DataQ,self.DataQ.ID) else !self.debugmsg('found dataq record for id ' & self.ViewQRightField & ', value was ' & self.DataQ.CurrValue) self.DataQ.CurrValue = choose(self.DataQ.CurrValue = 1,0,1) self.debugmsg('setting to ' & self.DataQ.CurrValue) self.DataQ.Changed = true put(self.DataQ) end self.setIcons() self.SaveCheckboxData() end end
Reconsidering the design
Aside from the lack of a consistent naming convention, this class has other problems. For one thing it combines user interface and database code, and while this is common practice in the Clarion world it does make class reuse and repurposing more difficult. What if you want to use a simple list box instead of an ABC browse? Or instead of saving changes to disk each time the user clicks a checkbox, save to disk once the user moves on to a different record in the left hand browse (perhaps with a confirm message)? And what if you want to save to some data store other than a file, say via a web service?
There are in fact at least three distinct spheres of operation at work, each of which should be separated into its own class or set of classes:
- User interface operations, including displaying the checkbox and allowing the user to change the checkbox.
- Storing the checkbox state
- Saving the linking data
This may be looking like a lot of work to replace a class that really isn't all that complex. You may be wondering if this work is worthwhile.
From personal experience I think it almost always is less work in the long run to break down any functionality into smaller, more testable and interoperable pieces. It's also almost always more work up front, but the result is code that's more reliable, flexible and maintainable.
And too many times I've looked at some code and thought "this easy to fix, no need to break it up into a set of classes" only to have the complexity grow over time to where I had to refactor anyway. By then I've typically invested wasted many hours keeping the code limping along.
Next time I'll start on the UI code refactoring.