The Problem with Embeds, Part 2: Refactoring the TXA Parser
- David Harms
- Geoff Robinson
- Mike Hanson
by Unknown user
In the first article in this series I said that most Clarion developers use embed points the wrong way, and by doing so they make their applications more difficult to maintain, test, debug and document. Almost every Clarion developer has done that; I've done it too. In these articles I intend to show how you can improve your code base by taking the majority of that code out of the embed points.
I'll be working through a couple of examples, including the Invoice app which ships with Clarion. But in this article I'll focus on the TXA embed parser example I introduced in Part 1.
In any analysis of embed code and how it might be better deployed, the first thing you need is a convenient way to look at just your embeds. That's not so easy because, embeds being embeds, they're sprinkled throughout your application.
Really you have two options: You can browse it in the embed list or in the embeditor, or you can use a tool to extract just the embeds so you can look at them without the distraction of the generated code.
About TXAs
As far as I know, there's only one way to programmatically extract the embed code from an APP. First, you have to export a TXA, which is an import/export text version of the information contained in an APP file. And second, you to parse the TXA to get the embeds, which can be a bit of a pain as the TXA format isn't documented.
A couple of years I wrote a TXA parser to do just that task, in support of an article series on the most popular embed points. So it seemed natural, when I needed a way to extract embed points for this series, to revisit that code.
Unfortunately, that code isn't very pretty. Most of it is contained in just a couple of embed points. In the TakeAccepted method's data section there are some declarations:
x long
vars group,pre()
procname string(200)
procFromABC string(60)
procCategory string(60)
embedname string(60)
embedPriority long
embedParam1 string(200)
embedParam2 string(200)
embedParam3 string(200)
whenLevel byte
end
dumptrace byte(0)
LastProcName like(procname)
lastEmbedName string(500)
currembedname string(500)
And then a little later on in TakeAccepted, at an embed that's called when the user presses the Import button, the TXA gets parsed. That code loops through the records in a previously created queue of TXA files (txaq):
?progressvar{prop:rangehigh} = records(txaq)
setcursor(cursor:wait)
loop x = 1 to records(txaq)
get(txaq,x)
?progressVar{prop:progress} = x-1
clear(ema:record)
EMA:TXA = txaq.name
Access:EmbedApp.Insert()
EmbedApp{prop:sql} = 'select last_insert_id()'
next(EmbedApp)
! Add the queue header record
access:TextFile.Close()
GLO:TextFileName = txaq.name
access:TextFile.Open()
Access:TextFile.UseFile()
set(TextFile)
ProcName = ''
state = 0
lineNo = 0
clear(procname)
clear(lastprocname)
clear(lastembedname)
clear(currembedname)
LOOP
next(TextFile)
if errorcode() then break.
dumptrace = false
lineNo += 1
CASE state
OF 0 ! search for the start of a module or procedure, or an embed
if sub(txt:rec,1,11) = '[PROCEDURE]'
clear(vars)
state = 10
elsif sub(txt:rec,1,8) = '[MODULE]'
clear(vars)
procName = '[MODULE]'
elsif sub(txt:rec,1,7) = 'EMBED %'
embedName = sub(txt:rec,7,len(txt:rec))
state = 30
elsif sub(txt:rec,1,8) = '[SOURCE]'
state = 50
end
OF 10 ! get procedure name details
if sub(txt:rec,1,4) = 'NAME'
procName = sub(txt:rec,6,len(txt:rec))
state = 11
end
do CheckForMissedEmbed
OF 11
if sub(txt:rec,1,8) = 'FROM ABC'
procFromABC = sub(txt:rec,10,len(txt:rec))
state = 12
end
do CheckForMissedEmbed
OF 12
if sub(txt:rec,1,8) = 'CATEGORY'
procCategory = sub(txt:rec,11,len(clip(txt:rec))-11)
end
state = 0
do CheckForMissedEmbed
of 30 ! Look for a first embed parameter
if sub(txt:rec,1,11) = '[INSTANCES]'
state = 41
elsif sub(txt:rec,1,8) = '[SOURCE]'
state = 50
end
of 41 ! Get first parameter
if sub(txt:rec,1,6) = 'WHEN '''
embedParam1 = sub(txt:rec,7,len(clip(txt:rec))-7)
WhenLevel = 1
!db.out('whenlevel=' & whenlevel)
end
state = 42
do CheckForMissedEmbed
of 42 ! Look for a second embed parameter
if sub(txt:rec,1,11) = '[INSTANCES]'
state = 43
elsif sub(txt:rec,1,8) = '[SOURCE]'
state = 50
end
of 43 ! Get second parameter
if sub(txt:rec,1,6) = 'WHEN '''
embedParam2 = sub(txt:rec,7,len(clip(txt:rec))-7)
WhenLevel = 2
end
state = 44
do CheckForMissedEmbed
of 44 ! Look for a third embed parameter
if sub(txt:rec,1,11) = '[INSTANCES]'
state = 45
elsif sub(txt:rec,1,8) = '[SOURCE]'
state = 50
!db.out('found PRIORITY')
end
of 45 ! Get third parameter
if sub(txt:rec,1,6) = 'WHEN '''
embedParam3 = sub(txt:rec,7,len(clip(txt:rec))-7)
WhenLevel = 3
!db.out('whenlevel=' & whenlevel)
end
state = 50
do CheckForMissedEmbed
of 50 ! look for the priority
if sub(txt:rec,1,8) = 'PRIORITY'
embedPriority = sub(txt:rec,10,len(txt:rec))
if lastprocname <> procname
! insert new EmbedProc record
clear(EMP:record)
EMP:Proc = procname
EMP:ProcFromABC = ProcFromABC
EMP:ProcCategory = ProcCategory
EMP:EmbedAppID = EMA:EmbedAppID
Access:EmbedProc.Insert()
EmbedProc{prop:sql} = 'select last_insert_id()'
next(EmbedProc)
lastprocname = procname
end
! Add the embed record
currEmbedName = clip(embedName) & clip(embedparam1) |
& clip(embedparam2) & clip(embedparam3) & embedpriority
if currEmbedName <> lastEmbedName
lastEmbedName = currEmbedName
EMB:EmbedProcID = EMP:EmbedProcID
EMB:Embed = EmbedName
EMB:Param1 = EmbedParam1
EMB:Param2 = EmbedParam2
EMB:Param3 = EmbedParam3
EMB:Priority = embedpriority
access:Embed.Insert()
end
state = 51
end
of 51
state = 60
do CheckForMissedEmbed
OF 60 ! capturing embed
! Quit when [END] encountered
if sub(txt:rec,1,1) = '['
if sub(txt:rec,1,5) = '[END]'
case WhenLevel
of 3
WhenLevel = 2
embedParam3 = ''
of 2
WhenLevel = 1
embedParam2 = ''
of 1
WhenLevel = 0
embedParam1 = ''
end
state = 0
elsif sub(txt:rec,1,8) = '[SOURCE]'
! look for another embed under this [EMBED] point
state = 50
else
! could be we're done
state = 0
end
elsif sub(txt:rec,1,6) = 'WHEN '''
case WhenLevel
of 0
! get the first param
embedParam1 = sub(txt:rec,7,len(clip(txt:rec))-7)
WhenLevel = 1
state = 42
of 1
! get the second param
embedParam2 = sub(txt:rec,7,len(clip(txt:rec))-7)
WhenLevel = 2
state = 50
of 2
state = 50
end
else
! write embed buffer
end
else
do CheckForMissedEmbed
END
END
Access:TextFile.Close()
end
?progressVar{prop:progress} = records(txaq)
setcursor()
In Embeds-original.zip have a look at the ImportTXAs procedure in Embeds.app for the complete source.
What I had in mind for my new utility was something more along these lines:
My original parser's functionality was overkill for this new app; I really didn't need to build up an elaborate database of applications, procedures, and embed points. I just needed a list of embed points. But obviously I needed all of the parsing capabilities, gnarly though the code might be.
Unfortunately, there wasn't any way to use my original code unchanged, primarily because that code didn't actually extract the embedded source from the TXA - there was no need to capture the actual embed code since I was just logging embed usage. And I was motivated not to store the embed code: I had asked for TXA submissions which could contain sensitive information, and I didn't want to accidentally expose anyone's embed code to public view.
So what were my options? A few came to mind:
- Just cut and paste the source. This is what a lot of us do, and it has the obvious drawbacks of creating multiple versions of the code to maintain. If I find a bug in my parsing code, I'll need to hunt down every place I've pasted a copy and make the change there.
- Put my original procedure in a DLL and call it as needed. In this case my DLL also presents a user interface. That would result in some UI clunkiness in the app I envisioned in Figure 1, which doesn't need to call yet another window just to do the parsing.
- Put the common source in source files and INCLUDE them. I could even include just portions of the files using labels. The drawback here is that there's no way to know how the various sections of source code might be used, or how any bug fixes to that source might cause unexpected bugs. Using INCLUDE statements this way results in an almost complete loss of control over the source code.
- Create a template containing the source code. This certainly helps keep the code in one place, but it has a lot of negatives when it comes to maintaining and testing that code since you have to put the template in an app and generate the code, and then you have to port any changes back to the template.
And there were other problems. Because my original app was tied to a particular data store (a PostgreSQL database), any re-use of that code would have to know the table definitions. Since Clarion only supports one dictionary per app, any apps that used this procedure would either need to use the dictionary or import the tables from that dictionary.
Class or procedure?
So what's the answer? If it's not a template, and not a multi-purpose generated procedure and not INCLUDEd source, what's left? Procedures and classes, that's what. But not just any procedures or classes. I wanted to write code that had as few dependencies as possible.
Some of the dependencies I wanted to avoid:
- Files/tables - not tie my code to a specific database
- Windows/controls - not tie my code to a specific user interface element
- Other code - keep calls to other procedures/classes to a minimum
So when should you use a class, and when a procedure?
In almost all cases a class is preferable to a procedure, in the same way that a procedure is almost always preferable to spaghetti code. A procedure presents a single point of entry and a single result. That's not to say you can't return multiple values from a procedure - you clearly can, as Steve Parker has showed. But procedures don't have the flexibility of classes.
In fact, a class method is really just a procedure, so using a class already gets you everything a procedure can do. But it also gets you more, because in a class multiple methods can operate on shared data.
The test app
In recent years I've become a devotee of test-driven development, which embodies the idea that you write your test first and then start in on the code needed to pass the test. I find this provides a lot of clarity to my class design.
So my first question is, what should my test(s) look like?
I'll need a parser object, of course. And this is going to be part of the DevRoadmaps Clarion Library, so it needs to follow the DCL naming conventions. That means that DCL_ is the first part of the name (Clarion doesn't support namespaces in class names, so I fake these by separating name parts with underscores).
The second part of the name? Maybe Clarion, since the parser is a class that relates specifically to Clarion's own functionality. If, for instance, I wrote some code to manipulate solution or project files I'd also begin those classes with DCL_Clarion_. If I'm not expecting a lot of classes then two levels of naming is sufficient, although sometimes I'll go to three.
Provisionally, I'll name my class DCL_Clarion_TXAParser. For more on creating test apps see Creating a ClarionTest test DLL (DCL).
The first test
So what should my first test look like? A reasonable place to start seems to be identifying the number of embed points in a TXA, using code like this:
txa DCL_Clarion_TXAParser
code
txa.Parse('test.txa')
AssertThat(txa.GetEmbedCount(),IsEqualTo(6),'Wrong number of embeds found in TXA')
I'm not creating this code from scratch; instead I'm refactoring a previously created set of classes to use the DCL standards and existing DCL classes. But the original code doesn't have a GetEmbedCount method so I'll have to add that at some point.
The class
The first decision I have to make is how to store the embed data. Originally I used a SQL database, but this now appears to be a liability. My parser should use something more transient that can be converted to a permanent store or just discarded after use. An in-memory data store, in the form of nested queues, fits the bill:
DCL_Clarion_TXAParser_ProcedureQueue queue,TYPE ProcedureName string(40) EmbedQ &DCL_Clarion_TXAParser_EmbedQueue END DCL_Clarion_TXAParser_EmbedQueue queue,type EmbedName string(100) EmbedLineQ &DCL_Clarion_TXAParser_EmbedLineQueue END DCL_Clarion_TXAParser_EmbedLinesQueue queue,TYPE Line cstring(1000) END
Since these queues are used by DCL_Clarion_TXAParser I've included them in DCL_Clarion_TXAParser.inc and have given them a prefix of DCL_Clarion_TXAParser_.
The parser's job will be to populate these queues so that I end up with a DCL_Clarion_TXAParser_ProcedureQueue containing one or more records. Each of those procedure records has one or more DCL_Clarion_TXAParser_EmbedQueue records, each of which has one or more DCL_Clarion_TXAParser_EmbedLineQueue records for each line in a given embed point. With that queue structure in hand I can easily update a database or create a text file, as I see fit.
At first blush this looks like an ideal task for a procedure. Pass in the name of the TXA and an empty queue and get back a filled queue: presto! But here's why I think the procedural solution is hardly ever a good solution: there's almost always some new functionality you will want to add which doesn't fit into the existing procedure.
Think about error handling. You can have the parsing procedure return an errorcode if the parse fails for any reason, but what if you want to get a more detailed error response? What if you want to enable tracing or logging? How would you do that in a single procedure call, without burdening the procedure with some obscenely large number of parameters?
In fact, once I began rewriting my parsing code as a class I ended up with rather a lot of methods and a few properties as well:
DCL_Clarion_TXAParser Class,Type,Module('DCL_Clarion_TXAParser.CLW'),Link('DCL_Clarion_TXAParser.CLW',_DCL_Classes_LinkMode_),Dll(_DCL_Classes_DllMode_)
ProcedureQ &DCL_Clarion_TXAParser_ProcedureQueue
ProcedureQIsExternal bool
Errors &DCL_System_ErrorManager
Construct Procedure()
Destruct Procedure()
AddNewEmbed procedure(string embed,string embedparam1,string embedparam2,string embedparam3,string embedpriority),private
AddNewProcedure procedure(string pname),private
CheckForMissedEmbed procedure(string s,long lineno,long state),private
GetEmbedCount procedure,long
GetProcedureCount procedure,long
Parse PROCEDURE(string filename),bool,proc
RemoveCurrentProcedureFromQueue procedure,private
Reset PROCEDURE
SetQueue procedure(DCL_Clarion_TXAParser_ProcedureQueue procedureQ)
End
I won't go into all of these methods in detail - you can have a look at the source if you're interested. Briefly, there are a few private methods to break up the internal code into reusable blocks (you'd use routines in a procedural implementation), and there are some public methods to get the procedure and embed counts, parse the specified TXA, reset the parser and specify the particular queue to use.
Now, show me how you'd implement all that in a procedure!
Note that use of the SetQueue method. By default the class will use its own internal DCL_Clarion_TXAParser_ProcedureQueue instance, but if I wish I can tell the class to use an external queue so I can more easily process the data, e.g. to write it to a database or a text file.