[GRASS-dev] Re: [GRASS-user] GRASS6.3 on Windows

[Glynn Clements]

Michael Barton wrote:

> > If you find yourself typing 130+ instances of near-identical code,
> > that should be a clue that you should add some more infrastrcture.
> > 
> > For a start, if you add:
> > 
> > proc errorBox {msg} {
> >     tk_messageBox -type ok -icon error -title [G_msg "Error"] -message $msg
> > }
> > 
> > you would be able to replace e.g.:
> > 
> > tk_messageBox -type ok -icon error -title [G_msg "Error"] \
> > -message [G_msg "Error creating tempfile: $error"]
> > 
> > (of which I see at least 102 occurrences) with just:
> > 
> > errorBox [G_msg "Error creating tempfile: $error"]
> 
> Agreed that this reduces the number of characters in the code. But it isn't
> much different timewise for cutting and pasting. It would make it easier to
> globally format all error messages in the future.

It isn't about the time required to enter the code in the first place. 
It's about being able to subsequently change the behaviour for all
instances by modifying a single function rather than 100+ copies.

IOW: abstraction. Keep the details of error handling in one place (or
a few places), and have the individual instances simply request that
an error be handled.

Beyond that, it's not uncommon for large projects to have substantial
hierarchies of "stub" functions for common operations, simply to
categorise the flows. E.g.:

	void handle_file_read_error(const char *msg) {
		handle_file_error(msg);
	}
	
	void handle_file_write_error(const char *msg) {
		handle_file_error(msg);
	}
	
	void handle_file_error(const char *msg) {
		handle_io_error(msg);
	}
	
	void handle_io_error(const char *msg) {
		handle_error(msg);
	}
	
	void handle_error(const char *msg) {
		/* the real work is done here */
	}

As it stands, this offers no advantage over either calling
handle_error() directly or, at the other extreme, simply inlining the
error handling.

But in practice, code seldom gets written then left untouched for the
rest of its days. When it comes to making changes, the above structure
means that you can change the handling of a particular category of
cases without being limited to a choice of either modifying
handle_error() (and affecting every case), or having to manually
modify individual occurrences.

> > Beyond that, the common error-trapping idioms for exec can be
> > incorporated into functions, e.g. (untested):
> > 
> > proc execNoError {cmd args} {
> > if {[catch {set result [eval exec [list $cmd] $args]} result]} {
> > tk_messageBox -type ok -icon error -title [G_msg "Error"] -message [G_msg
> > $result]
> > # force the caller to return
> > uplevel 1 return
> > }
> > return $result
> > }
> > 
> > This would allow simple cases to avoid explicit error handling
> > altogether.
> 
> In retrospect, it would have been better to have a single command processing
> function that used arguments or flags to merge the multiple ones now in the
> TclTk GUI (run, run_ui, execute, ...) and handle error trapping at the same
> time--as per Hamish' comments in a related thread and as we're doing with
> cmd.Command in wxgrass.

This is an area where a hierarchy similar to the above would be
applicable. A single level (one function called directly from
throughout the code) won't suffice. Even without the "term" case
(legacy programs requiring an xterm), you probably need one case for
display commands, one for "normal" computational commands run from the
menus, one for obtaining the interface description with --tcltk, etc.

At the lowest level, you would have one command, either with a lot of
options or which is little more than "exec" itself.

For any additional steps which it might perform, there is probably
some case where you would want to avoid that. To handle that, you
either provide options to enable/disable the additional functionality,
or have multiple higher-level functions which provide the additional
functionality, or some combination of the two. Specific cases either
use the appropriate set of options or call the appropriate
higher-level version.

E.g. for display commands, you probably wouldn't want errors to
reported through dialogs. If the user sets an option which causes most
display commands to fail, they won't want to have to individually
acknowledge a dozen error dialogs before they can correct the problem.

[Remember when web browsers would pop up an error dialog whenever an
error occurred? The current behaviour of reporting the problem in the
frame is a lot less annoying.]

-- 
Glynn Clements <glynn at gclements.plus.com>