The Problem with Embeds, Part 1: What Went Wrong?

by Unknown user

As Clarion developers we (well, most of us) live and die by embed code. Embeds are, after all, at the heart of Clarion's effectiveness. They're the primary way we add custom code to applications that are otherwise template-generated.

And if you're like most Clarion developers, you're probably using embed points the wrong way. Without realizing it you're making your applications more difficult to maintain, harder to test and debug, and almost impossible to document.

In this series of articles I'll analyze the embedded code in the Invoice example that ships with Clarion, and I'll show how significant portions of that application's embed code can be transformed into something that is maintainable, debuggable, even testable, and much easier to document.

The Clarion application architecture

We all use Clarion because in some way or another it makes our jobs as developers easier. There are still a few hand-coders in our midst who find sufficient benefit in the Clarion language itself, but for the rest of us it's all about the templates. And whether you're using ABC or Legacy (Clarion) templates, the great thing is that Clarion creates an entire application framework for you, largely based on the contents of your dictionary.

Application frameworks aren't talked about much in the Clarion world, mostly because we don't have to build them. They're supplied for us, and we've just come to accept them as the way things are.

The key features embodied in Clarion applications (leaving aside for the moment how they're built in the first place) include:

  • FILE structures to model files/tables
  • The database access grammar
  • VIEWs for more sophisticated data access
  • The browse/form approach to viewing and updating data
  • Queue-based browse paging
  • Reporting via the REPORT structure
  • WINDOW structures with built in data binding
  • The ACCEPT loop

The ABC library abstracts away some of this functionality, like the ACCEPT loop which is now hidden inside the WindowManager class. Similarly, the FileManager and RelationManager classes add a new layer on top of traditional data manipulation (although this abstraction is quite leaky, as when you have to issue a SET using the file access grammar before you can use the FileManager to retrieve data in key order).

The vast majority of Clarion applications in existence are AppGen creation; they embody these classic Clarion architectural features. Among other things that gives Clarion developers a common basis of discussion, and I don't think it's going too far to say that this shared application architecture is the main reason, or at least a very big reason, for the strength of the Clarion community over the last twenty years.

The problem with the Clarion architecture

But as useful as the Clarion architecture is, it has a problem. And that problem is going to grow in size as your applications grow in size and complexity.

The problem is embeds.

Not that there are embeds. We'd be lost without them. Embed points make it possible to plug our custom code into the almost any location within the standard Clarion architecture.

The problem is that embeds are almost always misused!

Example #1: The Invoice app

Here's an example from the shipping Clarion sample apps, specifically the Invoice app. The following is a listing of the embedded source from the UpdateDetail procedure, which lets the user add invoice line items:

PROCEDURE: UpdateDetail
 
  EMBED: %ProcedureRoutines 4000
 
  !Calculate taxes, discounts, and total cost
  !----------------------------------------------------------------------
  CalcValues  ROUTINE
      IF DTL:TaxRate = 0 THEN
        IF DTL:DiscountRate = 0 THEN
          DTL:TotalCost = DTL:Price * DTL:QuantityOrdered
        ELSE
          LOC:RegTotalPrice = DTL:Price * DTL:QuantityOrdered
          DTL:Discount = LOC:RegTotalPrice * DTL:DiscountRate / 100
          DTL:TotalCost = LOC:RegTotalPrice - DTL:Discount
          DTL:Savings = LOC:RegTotalPrice - DTL:TotalCost
        END
      ELSE
        IF DTL:DiscountRate = 0 THEN
          LOC:RegTotalPrice = DTL:Price * DTL:QuantityOrdered
          DTL:TaxPaid = LOC:RegTotalPrice * DTL:TaxRate / 100
          DTL:TotalCost = LOC:RegTotalPrice + DTL:TaxPaid
        ELSE
          LOC:RegTotalPrice = DTL:Price * DTL:QuantityOrdered
          DTL:Discount = LOC:RegTotalPrice * DTL:DiscountRate / 100
          LOC:DiscTotalPrice = LOC:RegTotalPrice - DTL:Discount
          DTL:TaxPaid = LOC:DiscTotalPrice * DTL:TaxRate / 100
          DTL:TotalCost = LOC:DiscTotalPrice + DTL:TaxPaid
          DTL:Savings = LOC:RegTotalPrice - DTL:TotalCost
        END
      END
  
  EMBED: %ProcedureRoutines 4000
  
    !Update InvHist and Products files
    !-----------------------------------------------------------------------
    UpdateOtherFiles ROUTINE
 
     PRO:ProductNumber = DTL:ProductNumber
     Access:Products.TryFetch(PRO:KeyProductNumber)
     CASE ThisWindow.Request
     OF InsertRecord
       IF DTL:BackOrdered = FALSE
         PRO:QuantityInStock -= DTL:QuantityOrdered
         IF Access:Products.Update() <> Level:Benign
           STOP(ERROR())
         END !end if
         INV:Date = TODAY()
         INV:ProductNumber = DTL:ProductNumber
         INV:TransType = 'Sale'
         INV:Quantity =- DTL:QuantityOrdered
         INV:Cost = PRO:Cost
         INV:Notes = 'New purchase'
         IF Access:InvHist.Insert() <> Level:Benign
           STOP(ERROR())
         END !end if
       END !end if
     OF ChangeRecord
       IF SAV:BackOrder = FALSE
         PRO:QuantityInStock += SAV:Quantity
         PRO:QuantityInStock -= NEW:Quantity
         IF Access:Products.Update() <> Level:Benign
           STOP(ERROR())
         END
         InvHist.Date = TODAY()
         INV:ProductNumber = DTL:ProductNumber
         INV:TransType = 'Adj.'
         INV:Quantity = (SAV:Quantity - NEW:Quantity)
         INV:Notes = 'Change in quantity purchased'
         IF Access:InvHist.Insert() <> Level:Benign
           STOP(ERROR())
         END !end if
       ELSIF SAV:BackOrder = TRUE AND DTL:BackOrdered = FALSE
         PRO:QuantityInStock -= DTL:QuantityOrdered
         IF Access:Products.Update() <> Level:Benign
           STOP(ERROR())
         END !end if
         INV:Date = TODAY()
         INV:ProductNumber = DTL:ProductNumber
         INV:TransType = 'Sale'
         INV:Quantity =- DTL:QuantityOrdered
         INV:Cost = PRO:Cost
         INV:Notes = 'New purchase'
         IF Access:InvHist.Insert() <> Level:Benign
           STOP(ERROR())
         END !end if
       END ! end if elsif
     OF DeleteRecord
       IF SAV:BackOrder = FALSE
         PRO:QuantityInStock += DTL:QuantityOrdered
         IF Access:Products.Update() <> Level:Benign
           STOP(ERROR())
         END
         INV:Date = TODAY()
         INV:ProductNumber = DTL:ProductNumber
         INV:TransType = 'Adj.'
         INV:Quantity =+ DTL:QuantityOrdered
         INV:Notes = 'Cancelled Order'
         IF Access:InvHist.Insert() <> Level:Benign
           STOP(ERROR())
         END  !End if
       END !End if
     END !End case
  
  EMBED: %WindowManagerMethodCodeSection Init (),BYTE 6500
 
    !Initializing a variable
    SAV:Quantity = DTL:QuantityOrdered
    SAV:BackOrder = DTL:BackOrdered
    CheckFlag = False
 
  EMBED: %WindowManagerMethodCodeSection Init (),BYTE 8030
 
    IF ThisWindow.Request = ChangeRecord OR ThisWindow.Request = DeleteRecord
      PRO:ProductNumber = DTL:ProductNumber
      Access:Products.TryFetch(PRO:KeyProductNumber)
      ProductDescription = PRO:Description
    END
 
  EMBED: %WindowManagerMethodCodeSection Init 4000
 
     !Updating other files
      DO UpdateOtherFiles
 
  EMBED: %WindowManagerMethodCodeSection 8500
 
    ! After lookup and out of stock message
    IF GlobalResponse = RequestCompleted
      DTL:ProductNumber = PRO:ProductNumber
      ProductDescription = PRO:Description
      DTL:Price = PRO:Price
      LOC:QuantityAvailable = PRO:QuantityInStock
      DISPLAY
      IF LOC:QuantityAvailable <= 0
        CASE MESSAGE('Yes for BACKORDER or No for CANCEL',|
                     'OUT OF STOCK: Select Order Options',ICON:Question,|
                        BUTTON:Yes+BUTTON:No,BUTTON:Yes,1)
        OF BUTTON:Yes
          DTL:BackOrdered = TRUE
          DISPLAY
          SELECT(?DTL:QuantityOrdered)
        OF BUTTON:No
          IF ThisWindow.Request = InsertRecord
            ThisWindow.Response = RequestCancelled
            Access:Detail.CancelAutoInc
            POST(EVENT:CloseWindow)
          END !If
        END !end case
      END !end if
      IF ThisWindow.Request = ChangeRecord
        IF DTL:QuantityOrdered < LOC:QuantityAvailable
          DTL:BackOrdered = FALSE
          DISPLAY
        ELSE
          DTL:BackOrdered = TRUE
          DISPLAY
        END !end if
      END !end if
      IF ProductDescription = ''
        CLEAR(DTL:Price)
        SELECT(?CallLookup)
      END
      SELECT(?DTL:QuantityOrdered)
    END
 
  EMBED: %ControlEventHandling ?DTL:QuantityOrdered Accepted 8800
 
    !Initializing a variable
    NEW:Quantity = DTL:QuantityOrdered
     !Low stock message
    IF CheckFlag = FALSE
      IF LOC:QuantityAvailable > 0
        IF DTL:QuantityOrdered > LOC:QuantityAvailable
          CASE MESSAGE('Yes for BACKORDER or No for CANCEL',|
                         'LOW STOCK: Select Order Options',ICON:Question,|
                           BUTTON:Yes+BUTTON:No,BUTTON:Yes,1)
          OF BUTTON:Yes
            DTL:BackOrdered = TRUE
            DISPLAY
          OF BUTTON:No
            IF ThisWindow.Request = InsertRecord
              ThisWindow.Response = RequestCancelled
              Access:Detail.CancelAutoInc
              POST(EVENT:CloseWindow)
            END !
          END !end case
        ELSE
          DTL:BackOrdered = FALSE
          DISPLAY
        END !end if Detail
      END !End if LOC:
      IF ThisWindow.Request = ChangeRecord
        IF DTL:QuantityOrdered <= LOC:QuantityAvailable
          DTL:BackOrdered = FALSE
          DISPLAY
        ELSE
          DTL:BackOrdered = TRUE
          DISPLAY
        END !end if
      END !end if
      CheckFlag = TRUE
    END !end if
 
  EMBED: %ControlEventHandling ?DTL:QuantityOrdered 6000
 
    !Calculate all totals
    DO CalcValues

To be fair this is actually a pretty old example, and I don't think it's one SoftVelocity has ever put forward as a best practices standard. It does, however, look a lot like a whole lot of Clarion code I've seen (and written), and it's probably pretty familiar to you as well.

Some of that code is appropriate for embeds. After all, where else are you going to respond to the user clicking a button, or move some data to the screen?

So, what's wrong with it? A few things:

  • It's difficult to code and to modify. You can't edit this code all by itself, the above combined listing notwithstanding; instead, you're dealing with different embed points at different locations in the generated source. The embeditor does make life a little easier, but you're still paging through code that really isn't relevant to the invoice business logic.
  • It isn't reusable. You can't easily take this code and use it somewhere else, say in a modified version of this form as required by a different client, or maybe in some back end code for a web app. 
  • It isn't testable. You really have no way of knowing if, say, the tax calculation code works other than to try out the form.
  • It's difficult to understand, in part because you have to infer what's happening by the location where code is executing. For instance, you only know that a block of code handles a quantity change because it's in an embed point for a quantity field, and then only because the field equate provides the clue. 

Yes, this example is a mess. Yet there's something quite valuable buried in all this code. It's not the database access; it's not the way the screen is updated. The valuable bit is the logic behind a particular way of handling invoicing.

Business logic: the real value of your application

Think of your application as made up of three parts:

  • the user interface
  • the data store (TPS files, SQL tables)
  • the business logic

That's an oversimplification because there can be some aspects of your application that don't fit nearly into those categories. But it's a useful categorization because it helps to focus attention on the part of your application that is truly unique.

What makes your application valuable to your clients? Is it the user interface (the UI)? Possibly, but unless you write your own UI layer then chances are other applications out there in your market segment that use similar controls, if in different arrangements. For most of us (especially as Clarion developers), the UI is not what makes our customers open their wallets.

Is it the data layer? If you use TPS files you have certain advantages and disadvantages compared to developers using other database systems, but unless you're selling specialized data access or data mining where the key is the speed and power of the database, it's not going to be your data layer that gives your application its unique value.

I think of user interface components and storage systems as commodities - they're things you buy, they're readily available, and while they have great utility they're also pretty much replaceable. Every business software development system I've seen gives you a way to create a UI and access data. Big deal. 

In almost all cases what gives a business application unique value is the business logic. That logic has to fit with how your customers do their business; the more flexible, the more reliable your business logic is, the more robust your application.

Imagine the Invoice.app is your own product (stay with me here). When you create invoices, applying taxes and discounts as appropriate to your industry, that's your business logic. When you manage inventory movement according to the unique requirements of your shipping system, that's your business logic.

And here's the thing: for the most part, Clarion isn't much help when it comes to writing code that embodies all that uniquely valuable business logic. 

That's right. Clarion is next to useless when it comes to creating the most valuable part of your application. It's no better than any other environment!

But Clarion is enormously useful at handling all the grunt work of creating menus, browses, forms, reports etc. It does all that work so you can devote most of your energy to writing the stuff that really matters. That's Clarion's true competitive advantage.

So, back to the Invoice app for a moment.

Somewhere, buried in that embed code for the UpdateDetail procedure, is a business process for handling invoicing. A quick glance wouldn't tell you that, and a closer look would probably leave you a little confused (at least it does me).

Extracting business logic

My task in this series of articles is to explore the ways that business logic can be teased out of the embed point and put into some code that's easy to understand, easy to maintain and modify, and easy to test.

Of course I'll still need those embeds so I can call that business logic, wherever it ends up. But the point of this exercise is to figure out how business logic can be coded for maximum benefit. Stuffing it all in embed points just has too many drawbacks.

I won't, however, be tackling the Invoice example until a little later on in this series. I've showed it to you because I think it's a fairly typical example, and I want you to know that's where these articles are headed. But it's also problematic because it's so tightly tied to the database and the UI.

Instead, I'm going to ease into this with a simpler example. And it's kind of a funny one.

Example #2: Embedded code to find embedded code

As I began to plan out this series of articles I realized very early on that I'd need a way to get a report on the embed code used by all the procedures in any given application. I decided to do this by exporting a TXA and then parsing that TXA, looking for the embeds.

This isn't an easy task. But as it happens I took it on some years ago when I ran a series of articles on the most popular embed points, based on data extracted from a number of reader-submitted TXAs. That code wasn't specifically concerned with the contents of the embeds, just whether or not they were used. But it wasn't that hard to adapt the code to extract the embedded source itself.

And here's the funny part. I wrote that original TXA parser as embedded code. Ha ha.

It's a mess.

Oh, the code isn't so bad. I only found a couple of bugs in it when I reworked it for this article series. But like the Invoice app, my embedded version of the TXA parser has some serious problems, including an awkward location, an inability to test, and tight binding to a particular database, making reuse in other situations impossible without significant modifications. Of course, at the time I couldn't imagine another place where I'd want to reuse this code, so why did I care?

Well, now I care.

Just remember - this is probably happening to you too, right now. You've got all sorts of code tucked away in your embed points, and much of it really doesn't belong there.

Figure 1 shows a screen shot of the embed list (slightly abbreviated) for the TXA parser procedure.

Figure 1. The original TXAParser procedure

There are just five embeds in the parser procedure. First, there's some global data:

! Queue to hold TXA list
txaq            QUEUE(File:queue),pre()  
                END
state           LONG(0)
LineNo          long

Then there's a line of code in the Init method to load up the queue of TXAs to import:

    !Get all files and directories
    DIRECTORY(txaq,'*.TXA',ff_:NORMAL)

There's one small routine that gets called regularly by the main import code, and which is really just a bit of debugging code:

CheckForMissedEmbed routine
    if sub(txt:rec,1,8) = '[SOURCE]'
        db.out('*** MISSED EMBED at line ' & lineno |
            & ', state ' &  state & ' ***')
    end

There's an embed for variables used in the parsing code:

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
LastProcName                like(procname)
lastEmbedName               string(500)
currembedname               string(500)

And finally there's one very large block of code to import the TXAs, which I won't reproduce in its entirety. It's basically a state engine that process the TXA in a loop and figures out where the embeds are and what they contain:

    access:TextFile.Open()
    Access:TextFile.UseFile()
    set(TextFile)
    state = 0
    lineNo = 0
    clear(procname)
    clear(lastprocname)
    clear(lastembedname)
    clear(currembedname)
    LOOP
        next(TextFile)
        if errorcode() then break.
        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
            
        ! OF cases for a bunch of other states


        of 50  ! Add the procedure and embed records
            if sub(txt:rec,1,8) = 'PRIORITY'
                embedPriority = sub(txt:rec,10,len(txt:rec))
                if lastprocname <> procname
                    clear(EMP:record)
                    EMP:Proc = procname
                    EMP:ProcFromABC = ProcFromABC
                    EMP:ProcCategory = ProcCategory
                    EMP:EmbedAppID = EMA:EmbedAppID
                    Access:EmbedProc.Insert()
                    lastprocname = procname
                end
                ! Add the embed record
                currEmbedName = clip(embedName) & clip(embedparam1) & |
                    clip(embedparam2) & clip(embedparam3) & embedpriority
                if currEmbedName <> lastEmbedName
                    ! Add the embed record yada yada
                end
                state = 51
            end

        ! OF cases for a bunch of other states

        END
    end
    ?progressVar{prop:progress} = records(txaq)
    setcursor()
    Access:TextFile.Close()

There's a ton more code, but you get the idea. This code is completely dependent on a specific database structure, and it references a control on the current window. It's sprinkled throughout a bunch of generated code. Those are all problems, to some degree. But mostly it's just hard to reuse this code somewhere else.

Making the TXA parser reusable

Now, how would I go about taking that parsing code and making it into something that could be used in that original embed analysis app as well as in the new embed extraction tool, the one that creates the listing from the Invoice app?

I really had three choices:

  1. Put the code in a procedure
  2. Put the code in a class
  3. Put the code in a template

You've probably heard the saying "the second time, it's a template." The idea is that if you find yourself writing the same code more than once, it really ought to go into a template.

The second time, it's not a template

The template advocacy statement I like is this one:

As long as your code isn't critical, unique business logic, the second time it goes in a template. Otherwise put it into a class, with one class per source file, using descriptive naming.

I'll come back to the descriptive naming later in this series. 

Templates aren't an optimal solution for most business logic, or really for any complex logic. If you try to treat the template language as a business programming language you'll quickly become frustrated by the quirky syntax, the need to mix Clarion language statements with template language statements and the difficulty of debugging.

The template language's job isn't to take the place of the Clarion language, it's to make it easy to generate repetitive Clarion code. And core business logic is generally not repetitive.

Clearly I'm going to write my core business logic as Clarion code. So that leaves procedures and classes.

Which approach did I choose? Read Part 2