Many-to-many checkboxes revisited, part 1: The original example

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:

!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.