How to set/get values when writing Classes?

TempoClock.default may be a good model for you here. (Actually I suggested this already.)

hjh

This works well, and is certainly the simplest solution… until you need to change the current style from another thread. Now this isn’t something you would do with TempoClock as this represents the tempo right now. But with GUI you might. This means you will have no idea what the value is at any given moment.

To combat this you can do what I suggested with a little addition of storing a current value for each thread, rather than a single value globally.

Again, if you are writing code for yourself, do what works, but as …

… you ought to consider how a user might use the code in ways you don’t expect.

GMProperty {
	var <value, listenerSlots;
	*new {|v| ^super.newCopyArgs(v); }
	connectToUpdate{
		|l|
		listenerSlots = (listenerSlots ? []) ++ [l].flat;
		[l].flat.do{|f| f.(value)};
	}
	updateValue { |v|
		value = v;
		listenerSlots.do{|l| l.(v) }
	}
}

GMStyle {
	classvar currentActiveStyles;
	var <mainColour, <secondaryColor;

	*new {|mainColour, secondaryColour|
		^super.new.init(mainColour, secondaryColour)}
	*setThreadStyle {|gmstyle|
		currentActiveStyles = (currentActiveStyles ? ())[thisThread.identityHash] = gmstyle}
	*rmThreadStyle {
		currentActiveStyles = currentActiveStyles.removeKey(thisThread.identityHash)}
	*safeStyle { |s|
		^s ?? {currentActiveStyles[thisThread.identityHash]}}
	init {|m, s|
		mainColour = GMProperty(m ? Color.red);
		secondaryColor = GMProperty(s ? Color.yellow);
		^this
	}
	withStyle { |f|
		var out;
		GMStyle.setThreadStyle(this);
		out = f.();
		GMStyle.rmThreadStyle();
		^out
	}
}

GMButton {
	var <but;
	*new {|style|
		^super.new.setStyle(GMStyle.safeStyle(style))
	}
	setStyle { |s|
		but = Button().states_([
			["yes", Color.grey, Color.white],
			["no", Color.white, Color.grey]]);
		s.mainColour.connectToUpdate({|v| but.states[0][1] = v });
	}
}

…and in the scd file…

~mainStyle = GMStyle(
	mainColour: Color.red, 
	secondaryColour: Color.yellow
);

{
b = ~mainStyle.withStyle({
     //this button will always use the mainStyle
	GMButton();
})
}.fork(AppClock)


~myBut.but.states[0][1] // the default value

~mainStyle.mainColour.updateValue(Color(0.3, 1.0, 0.2))

~myBut.but.states[0][1] // automatically got updated as it was connected to ~myStyle's mainColour

GMStyle.currentActiveStyles.postln;

Ok… Couple of suggestions though.

  1. currentActiveStyles ? () doesn’t short-circuit. I’d prefer currentActiveStyles ?? { () } instead.

  2. Hashes don’t guarantee uniqueness. It’s safer to use the thread itself as the dictionary key (i.e. just delete identityHash everywhere). IdentityDictionaries use identityHash internally but using this as the key isn’t exactly the same.

hjh

I tired using the thread itself but something odd was happening when I tried to delete the key… to be honest I just stuck identityHash in and it work so I stopped looking.

“Objects which are identical === should have the same hash values”, (help for Object.identiyHash) so I think it should work properly, although a thread might be reused, but that shouldn’t matter as they always outlive the coroutine (I think).

Hm, I’m at a loss what that could be. In an IdentityDictionary, any object can be used as a key, as long as you’re matching by identity (which you would certainly want to do, when looking up threads).

I tried it, seems OK to me.

// make a few routines
~routines = Array.fill(5, { |i| Routine { i.postln } });

// put them in a dict, with colors
~dict = IdentityDictionary.new;
~routines.do { |r| ~dict.put(r, Color.rand) };

~dict

~dict.size  // 5

// ok, let's arbitrarily 'stop' / release one of them
// 'take' just removes it from the array
r = ~routines.take(~routines.choose);

~dict.removeAt(r);  // or ~dict[r] = nil would also work

~dict.size;  // 4

~dict[r]  // nil
~dict.keys.includes(r)  // false

So at the end of this, whichever routine was chosen as r is definitively gone. Do it 5 times, ~dict will be empty.

Oh, I guess I see what the problem is. It should be removeAt. removeKey comes from some extension – I don’t have it, so I have no idea what it’s supposed to do, but clearly it doesn’t fully remove.

The problem with hashes is that objects which are not identical may have the same hash values. For instance, if you have an array of four integers, that’s 128 bits of information or 2**128 ~= 3.4e+38 possible values. If you hash that down to a 32-bit integer, then there are 2**32 ~= about 4.3 billion (4.3e+9) possible values. For every possible hash, then, there are 2**96 possible input arrays that will collapse down to the same hash. If you were storing data in a dictionary based on hashing a 4-int array, eventually you would have hash collision and loss of data.

Hash algorithms are designed to reduce the likelihood of collisions (and to spread apart the hashes for close-together values), but hashing is not the same as lossless compression. Information is lost and there is always the possibility of collision. Agreed that your example would probably work in the overwhelming majority of cases, but it isn’t guaranteed.

(Incidentally, behind the scenes, IdentityDictionary already hashes the key, using the same function that provides identityHash.)

A couple more thoughts about thread safety.

It’s true that, whenever possible, designing for thread safety would avoid the (nasty) problems of retrofitting for thread safety.

My thinking the other day was that, if the main design driver is “set it and forget it” for graphical styling, then the prospect of multiple threads creating multiple views, all with different styles, would be relatively unlikely (though not impossible). Those design ideals are somewhat at odds.

Another way to do thread safety would be to make the style a property of a thread-like object, something like:

GMTask : Task {
	var <>style;
	
	*new { |style, func, clock|
		^super.new(func, clock).style_(style)
	}
}

GM {
	classvar <>default;
	... styling stuff...
	
	*current {
		^thisThread.threadPlayer.tryPerform(\style) ?? { default }
	}
}


// user code:
GMTask(myGMStyle, {
	... GM guis here don't need to reference the style
	... their constructors could just say `GM.current`
}).play(AppClock);

Users would have to be aware to use GMTask (documentation responsibility), but this would eliminate the risk of leaking Routine instances in the collection (since there would be no collection). (For instance, if f errors out, then rmThreadStyle would never happen = object leak.)

hjh

AHH thanks! Didn’t notice removeAtKey was nonstandard, assumed it would remove the key-value pair at the key, rather than just the key.

Hadn’t thought of inheriting from task (I’ll definitely being this from now on, so thanks!), that is quite elegant! I much prefer this rather than accessing the classvar, the syntax looks more like SynthDef and other classes that take functions, and therefore more idiomatic to the language…

Anyway, this thread was about getting and setting values. Think we demonstrated some of the design issue that arise with such a simple task when building something for use outside your own code base.