Versions Compared

Key

  • This line was added.
  • This line was removed.
  • Formatting was changed.

...

It was time to revisit John's changes. As usual, one thing led to about five others. 

An all-source ClarionTest

...

But as I thought about this, I realized that ClarionTest really does belong as source code. It exists because I was fed up with having my business logic buried in embed points, and not only did I want to move that logic into reusable classes I also wanted to be able to test that business logic in some automated way. 

There were some odd contradictions in But while the first versions version of ClarionTest . I helped me achieve that goal, I soon realized that most of the business logic in ClarionTest itself was embed code. Perhaps that was an inevitable bit of bootstrapping, but once I had ClarionTest it made sense to move the app's own logic to ClarionTest, test it with ClarionTest, and then replace the ClarionTest embed code with the classes. 'Did that make any sense?Got it? 

That was step one in the ClarionTest reclamation project.

The more classes I write, and the more I learn about how I can write classes (there is always more to learn), the less satisfied I am with how the ABC class library (the foundation for the ABC templates) does things. That doesn't mean ABC is terrible code - it does lots of things well. But it's a pretty old design, and it does have some code smell. 

...

Given that premise, I ripped almost everything out of the ClarionTest source except for the window definitions and the data declarations necessary to compile those windows. And I started over.

...

 

Note

Don't for a minute think that whatever I'm describing in the following text is the "final" version of ClarionTest. If anything, ClarionTest has been, is and will be in a state of evolution. There are compromises and failures in the current code. When I notice them I'll comment on them, and try give some reason for my choices. And I'll come back to the code periodically and complain about the crap I wrote last time. To paraphrase Anil Dash, writing code is all

...

about trying to suck less. That doesn't mean I'm not ever happy with my code - I frequently sit back and look at something I've written with a great deal of satisfaction. But I know that just because it looks good now doesn't mean it will look equally good down the road.

...


Starting over (again and

...

again)

After commenting out all of the existing code and many of the data declarations, I had a bare bones, non-working app. So what were my requirements?

  • Be able to load up a test DLL
  • Remember the last test DLL loaded
  • Mark the individual tests that should be run, or run all
  • Display a summary of how many tests have failed and how many have passed
  • Option to display only the failed tests
  • Fix resizing so the last position/size is remembered

Reviewing the code

Diving into the ClarionTest source I noticed an implicit variable right off the bat. It was a simple one - just N#, hard to mess up. But implicits always worry me, and I already had an X variable so I changed all N# to X (after making sure I didn't have any nested loops where I'd already be using X). 

I also removed the XML configuration code (because it  and the web-style links, replacing them with buttons. 

Then I really started hacking away at the code. ClarionTest started out as an ABC app, which is really overkill for something that just shows a window with a few controls and does some pretty basic UI handling. There's all sorts of code there that simply isn't necessary, and again over time I would like ClarionTest itself to be something of a showcase for how you can build up an app using classes. 

...

An accept loop

For the last couple of years I've been consulting on a very large Legacy system. And while Legacy is painful to work with, there is one thing I really do appreciate about it: there's an actual Accept loop in the generated code.   I sometimes think it was a mistake for ABC to bury that little bit of Clarion brilliance in the WindowManager.Ask method:

Code Block
WindowManager.Ask PROCEDURE
  CODE
  IF SELF.Dead THEN RETURN .
  CLEAR(SELF.LastInsertedPosition)
  ACCEPT
    CASE SELF.TakeEvent()
    OF Level:Fatal
      BREAK
    OF Level:Notify
      CYCLE                 ! used for 'short-stopping' certain events
    END
  END

There's nothing in the loop itself except for a TakeEvent() call and the break and cycle options. I think it would make ABC code a lot more readable if procedures still had an Accept loop with those few lines of code. But maybe there are other reasons I'm not aware of. 

Maybe it's nostalgia, and maybe I'll regret not using WindowManager for ClarionTest, but for now I'm going to go with a regular old Accept loop and see where it takes me. If that means coming up with my own WindowManager replacement, I think I'm up for that. 

Resizing

For no particularly good reason I decided to tackle window resizing first. The first step was to find a way to remember the last window size and position.Here I did go with an ABC option: INIClass. 

INIClass is an example of a relatively benign ABC class. Although it conforms to the ABCInclude convention (which I generally despise), and although it's in a file with a bunch of other classes rather than in its own file (which I abhor) it does have the merit of being independent of any other ABC classes. And it provides a convenient way to store/retrieve configuration data. 

I added a relevant global include statement:

INCLUDE('ABUTIL.INC'),ONCE

and a global declaration followed by some global init and kill code:

Code Block
Settings                                      INIClass                              ! Global non-volatile storage manager
	CODE
    Settings.Init('.\ClarionTest.INI', NVD_INI)                ! Configure INIManager to use INI file
	Main()
    Settings.Kill                                              ! Destroy INI manager

The second parameter to Init is unnecessarily cryptic. I can deduce the _INI part, but what does NVD stand for? And why the need for an explicit call to Kill? Here's the method code:

Code Block
INIClass.Kill PROCEDURE
critProc  CriticalProcedure
  CODE
  critProc.init(SELF.critSect)
  IF  NOT SELF.Sectors &= NULL
    SELF.UpdateQueue('__Dont_Touch_Me__','Sectors',SELF.Sectors,SELF.Sectors.Family,SELF.Sectors.Item,SELF.Sectors.Type) !***
    DISPOSE(SELF.Sectors)
    SELF.Sectors &= NULL
  END
  RETURN

Blech. There's that infamous '__Dont_Touch_Me__' string that shows up in every INI file. And I still don't see the rationale for an explicit kill - if it hasn't been done at destruct the class should be smart enough to do it. 

Oh well. I'll hold my nose on this one until I get around to writing something that suits me better. 

Here's my Accept loop so far:

Code Block
	open(window)
	Window{PROP:MinWidth} = 600                              ! Restrict the minimum window width
	Window{PROP:MinHeight} = 300                             ! Restrict the minimum window height
 Resizer.Init(AppStrategy:Spread) ! Controls will spread out as the window gets bigger
	ACCEPT
		case event()
		of EVENT:OpenWindow
			settings.Fetch(ProcedureName,window)
 of EVENT:CloseWindow
			settings.Update(ProcedureName,window)
		of EVENT:Sized
			Resizer.Resize()
		END
		case accepted()
		of ?Close
			post(event:closewindow)
		END
		
	END
	close(window)




WindowResizeClass. I might want to replace this at some point, but I find it a relatively 

 

 

Next up: some code to read a directory. You know the drill. Declare a queue:

...