>>Well, if you are happy
>>
>>some thoughts
>>
>>(1) I have a problem with.
>>

>>luNewKey       = evaluate(m.lcAlias + "." + m.lcParentKeyExp)
>>
>>What if lcParentKeyExp is bintoc(cust_id)
>>

>>
>
>Valid point, I haven't thought about it, since in our system all keys are simple keys. I guess it would be simpler to switch to the table (select it), evaluate, then switch back instead of trying to parse the expression.
>
>>(2) I see you have built the table with ataginfo(). You can take it a step further (one of prev messages) (now you scan two tables, then you would scan only one)
>>Build one table which contains a field : Memo, Function ToCall: Cascade_parent('bintoc(fld,2)', '....', '')
>>
>>for simplicity and readbility
>
>I will still have to evaluate it in run-time, so I don't see it as a big improvement.

Yes, but you skip a whole bunch of tests and avoid two scan loops


>
>>Why not call all the necesary functions dependening on the table and triggertype ? That would take out a lot of spaghetti
>>Each function will test if it must do anything (keyexpr changed) and gracefully exit
>>
>>

>>select Into array (table , triggertype)
>>for
>>   if( !eval(rtrim(aa[m.i])) )
>>      Success = FALSE
>>      exit
>>   endif
>>endfor
>>

>>
>
>I was thinking last night about some further optimizations of the code. Basically, I believe, that this statement is true (though I'm going to test it). If our table is a parent table, then there is only one key field, that we need to test, e.g. there could be only one primary key, and it is involved in relations. Am I right?
>
>>( I do not think that the following line takes too much time (could be wrong though)
>>

>>if ( eval(keyExpr) == oldval(keyExpr) )
>>

>>
>I think it did. At least my code is now running about 100 times faster than before.

It would be evaluated once per relation involved if and only if two relations share the same key expr.
I suspect the factor 100 is not a result of that. But anyway



>
>
>>(3) break it down, ie why not call a function to handle the relation ? >> more readable
>>sample
>>

>>*---------------------------------------------------------------------------
>>function _Trigger_Process_Relations(_table, _type)
>>
>>	local Success
>>	Success = TRUE
>>	
>>	local TriggerArray[1], n, i
>>	n = 0
>>	
>>	do case
>>	case !m.Success
>>	
>>	case !GLOBAL_TRIGGERNODE.TriggerArray_Get(m._table, m._type, @TriggerArray, @n)
>>		Success = FALSE
>>	
>>	endcase
>>	
>>	for i = 1 to m.n
>>		
>>		do case
>>		case !m.Success
>>			exit
>>		
>>		case !_TriggerHistoryAdd(TriggerArray[ m.i ])
>>			Success = FALSE
>>			
>>		case !eval(TriggerArray[ m.i ])
>>			Success = FALSE
>>		
>>		case !NO_ERROR
>>			Success = FALSE
>>			
>>		endcase
>>		
>>		assert (m._table == CursorGetProp('SourceName')) message 'Not on table ' + m._table
>>
>>	endfor
>>	
>>	return (m.Success and NO_ERROR)
>>	
>>endfunc
>>*---------------------------------------------------------------------------
>>

>>
>
>Not sure I understood all these ideas. Anyway, I don't want to put too many changes. I already re-worked the original code a lot.


Just saying to add another function to the main to put the loop in. It adds to the readability
Just a for/next in not too many lines


>>(4) why all that testing at the beginning ?
>>
>>vartype(m.tcTriggerType) = 'C'
>>
>
>This piece I'm going to change slightly. But what if my code is called _ri_handler() instead of _ri_handler("UPDATE")?
>
>>if the trigger in the table of say customer for insert would be Trigger('CUSTOMER', 'I')
>>then the main entry point would be
>>
>>__RI_Handler(tcTableName, tcTriggerType)
>>
>>
>>(5) throw the code away. Write your own. And include support for compound keys. Not very difficult
>>___________________________________________
>
>They are already supported indirectly.

How is that ?

Success,