General developer forum

In large systems, even action-at-a-moderate-distance is a major problem. I am perpetually wary of inheritance, which is the main source of magic action-at-a-distance.

I am also wary of Exceptions, unfortunately. I am now coding a lot of code that has to handle every possible error condition, and exceptions are a pain to deal with - I have to walk down every damn library used and come up with a manifest of all the possible exceptions thrown. And if a library I use changes the libraries it uses internally, new exceptions might be thrown that I have no control of. Ugly long term maintenance story.

Both inheritance and exceptions look good in the small cases, but in large systems... homeopatic doses only...

Used well OO can give spectacular results and wonderul transparency. However, it is a big stick and as such it can do terrible damage. I've seen it used innapropriately many more times than appropriately.

It's simply a matter of balance - Moodle has been good at that in the past. Let's just make sure we don't have "Yay, PHP5 has all this cool stuff. How can we use it". Simplest is, nearly always, best.

As an aside, I confess that I hadn't heard the expression "action at a distance" before. It wonderfully describes the problem though

Sounds like a good plan, I should finish the proposal and code samples for File API sometime next week then we could start working on this in parallel.

There is a trend to wrap anything 3rd party in special Moodle layer, this keeps the API simpler, allows upgrades of 3rd party code without breaking public Moodle API and it would imho also allow us to rethrow exceptions to match Moodle conventions. Both DDL and DML already do this, I suppose portfolios and file storage will do the same.

Programming without exceptions is absolutely *** **** insane unless you hold an incredibly tight rein on every single developer to ensure that every single error condition is checked at every point. Which, if you do that, is fine, but this has not typically been the case in Moodle development.

(Did you spot the understatement in my last paragraph? Look carefully! It's there. )

This is why (just for example) backup/restore is a continuing maintenance disaster, especially for anybody who uses or maintains custom modules. At present, if anything or everything fails in backup, the most likely outcome is that there is no error message output and the system says that backup completed succesfully. Great result! Until you try to restore it, that is.

This is all caused because some functions return false and some other functions ignore that return value. Making it work involves correctly implementing a massive tree of return checks and horrible display in all different places, each of which needs to check for BACKUP_SILENT or whatever it is and maybe suppress the message, and so on.

A lack of exceptions is also why, for example, we (OU) had a serious bug with contexts recently. The build_context_path function works using a 'temporary' (but actually permanent...) table mdl_context_temp. Before each run this is wiped using TRUNCATE mdl_context_temp. Fine right? Now for various reasons this doesn't work on our particular Postgres configuration (you need to do DELETE FROM). But, because the database functions only return error codes instead of throwing exceptions, and because obviously nobody was going to check for error codes when doing TRUNCATE because come on how on earth could that ever fail, this was happening silently and causing severe problems (we moved things to other category, context gets updated correctly to new category... then a few hours later cron updates it back again).

If we'd had exceptions, this would have thrown an exception which (presuming nothing else caught it) would just display in the cron logs, or when running cron manually. We would have spotted it before the code even went live.

Exceptions are a life-saver: if something fails you know it and can fix it. And there's absolutely no reason to identify every piddling type of exception or maintain any kind of list; if you are catching problems and checking exception types, just throw in a general catch last.

For that reason it's probably not necessary to do anything specific with third-party exceptions (some other kind of error occurred? ok let's display it, end of). However if this is necessary (it is in Java and other languages where you have the concept of 'checked exceptions' ie Java will require you to catch any exception that might possibly be thrown or else you get a compile error) then it isn't very difficult. Here's some typical java code:

try
{
xmlLibrary.doXmlStuff(f);
}
catch(XMLLibraryException e)
{
throw new MyProgramException(e);
}

This uses the concept of a 'cause'. Is that in php exceptions? They were mostly taken from java so I assume so... if not it's easy to include in any specific exception type. Basically it just means that the original exception is stored as a member variable inside your own exception type, so that when you display the backtrace, you can include the trace from the original exception as well as the place where you re-threw it.

I've actually found it quite rare that you need to catch specific exceptions for different handling (i.e. where you have a try/catch block with more than one catch, and they aren't all doing the same thing). There are only a few special cases where that is often required (e.g. treating FIleNotFoundException differently from a more general IOException, that kind of thing).

--sam

MSSQL supports transactions for 'create table' etc as far as I'm aware, so it's only MySQL (?) and Oracle that don't. (Actually are people sure the latest Oracle version still doesn't? I should ask Rod here, he's supposedly been trained on it, but not sure if they covered that, it was only a week...)

That's a good point though - I think actually you would need to specify when constructing a transaction whether it is a 'DDL transaction' (in which case, don't bother trying to do it on Oracle/MySQL). Obviously could just be an =false parameter so normal transactions would be fine.

ADODB does have nested transaction support. I attached transaction_wrapper.php so you can see how it works if you want, not very complicated. (Some of the code is a bit of a mess but whatever.)

And I don't know about core moodle, but OU moodle does use transactions on updates. One other important part is restore - if restore breaks you end up with a half-created site, which is horrible. Don't think core moodle has a transaction there (we do).

Deleting courses would be good case for a transaction too, also creating and deleting activities (so you can't end up with course-module but no activity table entry or vice versa), etc etc.

--sam

+1 for Exceptions

I have been playing around with them in some of my more recent code and found them to be quite useful. Probably on everyone's mind already, but would be great if the exception class would also print a debug traceback if debugging is turned on. Also, I used the error code parameter as a flag for the type of string being passed. Zero was use the string in a get_string() call and 1 was use as is. This was for a module though, so the get_string() call was always for the module, but might be handy for quickly writing generic exception errors.

Since we are talking about PHP5 coding standards, has anyone thought about using __autoload()? Might help to reduce Moodle's memory usage and would probably require a naming convention for classes like how Zend Framework names the classes based on the directory structure and the file names.

I think I'd just do it like

MoodleException($errorcode,$module,$a=null)

then if an error occurs it does get_string('err_'.$errorcode,$module,$a) but if get_string isn't available it could just 'Error $errorcode occurred in module $module'. Link to moodledocs based on error code could still work...

I.e I wouldn't make people include an english version of the string, just display the language string name basically. After all that kind of exception should never really show to users. (Yeah, I know, it will, but not very often, and if the system is that broken, we should probably be using our separate 'apology server' - love that name - to give people a generic 'sorry it's broken' page rather than letting moodle render anything at all.)

...actually it occurs to me, even without complicated load-balancer installations and other non-Moodle stuff, maybe exceptions that do occur before setup.php is triggered should in fact always show a generic user-friendly 'we're really sorry ' page anyway. One trick we did in old systems is to always print the real error message in an html comment So users get the terribly polite, helpful, totally uninformative error, while a developer can just view source and find out what's wrong straight away.

--sam