FunctionList fails if removing before adding

Hi There,

I observed something that seems strange to me but not sure whether it’s a bug or feature. It involves FunctionList and goes something like this:

*** Welcome to SuperCollider 3.12.2. *** For help type cmd-d.
2022-12-16 10:22:06.145 sclang[70821:86146849] ApplePersistence=NO
sc3> a = FunctionList.new;
-> a FunctionList
sc3> f = { |b| b.postln };
-> a Function
sc3> a.removeFunc(f)
ERROR: Message 'at' not understood.
RECEIVER:
   nil
ARGS:
   Integer 0
CALL STACK:
	DoesNotUnderstandError:reportError
		arg this = <instance of DoesNotUnderstandError>
	Nil:handleError
		arg this = nil
		arg error = <instance of DoesNotUnderstandError>
	Thread:handleError
		arg this = <instance of Thread>
		arg error = <instance of DoesNotUnderstandError>
	Object:throw
		arg this = <instance of DoesNotUnderstandError>
	Object:doesNotUnderstand
		arg this = nil
		arg selector = 'at'
		arg args = [*1]
	Interpreter:interpretPrintCmdLine
		arg this = <instance of Interpreter>
		var res = nil
		var func = <instance of Function>
		var code = "a.removeFunc(f)"
		var doc = nil
		var ideClass = nil
	Process:interpretPrintCmdLine
		arg this = <instance of Main>
^^ ERROR: Message 'at' not understood.
RECEIVER: nil


sc3> a.addFunc(f)
-> a FunctionList
sc3> a.removeFunc(f)
-> nil
sc3> a.removeFunc(f)
-> nil

Essentially calling removeFunc before calling addFunc causes an exception.

If I add a function (any function in fact) and then call removeFunc (as often as I like), no exception is thrown.

Is this desired behaviour? The FunctionList | SuperCollider 3.12.2 Help documentation does not really go into.

IMHO calling removeFunc before or after first calling addFunc should not generate an exception but perhaps there is reasoning I do not understand! :thinking_emoji:

I believe the intended use is not to use FunctionList directly, but rather implicitly.

f = { |x| x.debug("function f") };
g = { |x| x.debug("function g") };

l = f;

l.value(1);

// note that removeFunc will work if l is still nil (no functions added)
l = l.removeFunc(f);  // nil, OK

l.value(2);  // nil

l = l.addFunc(g);

l.value(3);

l = l.addFunc(f);

l.value(4);

l.removeFunc(g);

l.value(5);

hjh

In the description (FunctionList | SuperCollider 3.12.2 Help) of the class there is this example:

a = FunctionList.new;
fork { loop { 0.7.wait; a.value.postln } };
a.addFunc({ 800.rand });
a.addFunc({ "another".scramble });

If direct use isn’t intended, then that example should be removed …

I’d rather have the method return a bool indicating whether the removal was successful, returning false if the function wasn’t present. Throwing when nothing should have changed seems wrong to me to.

It does return nil when nothing happened but throwing an exception is definitely strange.

I would create an issue over at Supercollider GitHub if this seems to be a language bug.

Looking at the implementation of FunctionList:

FunctionList : AbstractFunction {
	var <>array, <flopped=false;

	*new { arg functions;
		^super.newCopyArgs(functions)
	}
	addFunc { arg ... functions;
		if(flopped) { Error("cannot add a function to a flopped FunctionList").throw };
		array = array.addAll(functions)
	}
	removeFunc { arg function;
		array.remove(function);
		if(array.size < 2) { ^array[0] };
	}
 ... rest removed ...

What seems to be happening is that initially instance variable array is nil and the removeFunc will happily array.remove (i.e. nil.remove works) but nil.size < 2 is also true so it then tries to return nil[0] and hence the exception.

doing addFunc first initialises the array instance variable to be an array (or something other than nil).

That’s definitely a bug, wrong if condition.

My main point is that you wouldn’t encounter the exception when using it implicitly. (The bug should still be fixed, but there is a usage pattern that avoids it.)

hjh

Perhaps…

^super.newCopyArgs(functions ?? {[]})

… is the fix?

I believe it was done this way to avoid creating garbage collection load until there is actually something to store.

hjh

That does fix it, just tried it out.

Also if FunctionList is not intended to be used directly, then FunctionList.new should perhaps be “private” or throw an exception. I was just copying the example from the description and then had a weird combination of addFunc and removeFunc - definitely an edge case.

Edit: I changed my code from a = FunctionList.new to a = FunctionList.new([]) and am moving to bright skies :slight_smile:

For completion, created an issue over at GitHub