...
by Unknown user
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:
Code Block |
---|
!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:
Code Block |
---|
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.