A convenient class to use MLP regressor

Well, then we’re all in trouble:

b = Buffer.read(s, pathToAWavFile);
// at this point, what's the buffer size?

So @kesey , my suggestion for this specific class remains: put an action function in and call it a day. That’s consistent with usage throughout SC. It’s not a “cool” solution but it does work.

hjh

Thank you very much @jordan and @jamshark70 for your advices.

I added a callback method to the class (action argument) and I use forkIfNeeded everywhere I used to fork I don’t use CondVar inside the class cause I thaught that you can use it outside:
you make an instance, put a cond.wait just after (you have to be in a routine) and do a cond.signalOne inside your action method. This way you know that your object is ready.
Does that seem acceptable to you?

this is the new class:

RegAutoMapSrvLL {
	var <server,
	<synth, // Synth or Ndef instance
	<paramScale, // IdentityDictionary of \paramName = [minVal, maxVal, curve] ex: IdentityDictionary.newFrom([\freq, [20, 20000, \exp], \pan, [-1, 1], \level, [0, 10]])
	<paramExclude, // Array of parameter name to exclude
	<defaultName, // String name of the Window and name of the default loaded files
	<defaultPath, // String ex: "/home/fabien/Bureau/"
	<xCCNum, // control change Number
	<yCCNum,
	<midiChan, // midi channel Number
	<group,
	<bgColor, // Color of background
	<action, // A callback Function will be passed this as an argument when evaluated
	<window,
	<regScaler, // Synth(\regScaler1001)
	<paramArr, // Array of parameter name without the excluding ones
	<outputNumber, // Number of output
	<controlBus, // get a multi channel Bus
	<subBusDict, // IdentityDictionary of \parameter = Bus
	<xydata,
	<paramsdata,
	<xybuf,
	<paramsbuf,
	<mlp,
	<midiFunc,
	<>active, // Boolean for soft take over on xy midi mapping
	<xyModel, // IdentityDictionary of \x = value and \y = value
	<viewElement, // IdentityDictionary of viewElement[\controlName] = Button (for example)
	<live, // Boolean liveMode state (when live == true you save on defaultPath ++ defaultName file)
	controller,
	oscFunc,
	currentValues,
	isNdef;

	// Class Method

	*new { | server, synth, paramScale, paramExclude, defaultName, defaultPath, xCCNum, yCCNum, midiChan, group, bgColor, action |
		if (server.isKindOf(Server).not, {
			^warn("Server required");
		});

		if (server.serverRunning.not, {
			^warn("Server is not running");
		});

		paramScale = paramScale ?? { IdentityDictionary.new; };
		paramExclude = paramExclude ?? { Array.newClear; };

		paramExclude = paramExclude.collect({ arg paramName;
			paramName.asString.toLower;
		});

		paramExclude = paramExclude ++ ["in", "out", "input", "output", "inbus", "outbus", "bus", "doneaction", "trigger", "trig", "gate", "t_trig", "t_gate", "loop", "mute", "on",  "off"];

		defaultPath = defaultPath ?? { Platform.userHomeDir +/+ "Bureau/"; };
		defaultPath = defaultPath.asString;

		bgColor = bgColor ?? { Color.white; };
		action = action ?? { { arg reg; format("% is ready\n", reg).post; } };

		^super.newCopyArgs(server, synth, paramScale, paramExclude, defaultName, defaultPath, xCCNum, yCCNum, midiChan, group, bgColor, action).prInit;
	}

	// Instance Method

	remove {
		if (window.isClosed.not, { window.close });
	}

	getPreset {
		var preset, argString;
		argString = "";

		paramArr.do({ arg param, i;
			argString = argString ++ "\\" ++ param.asString ++ ", " ++ subBusDict[param].getSynchronous.asString;
			if ((i + 1) < paramArr.size, {
				argString = argString ++ ", ";
			});
		});

		if (isNdef, {
			preset = "Ndef(\\" ++ synth.key.asSymbol ++ ").xset(%);".format(argString);
		}, {
			preset = "Synth(\\" ++ synth.defName.asSymbol ++ ").set(%);".format(argString);
		});

		^(Post << preset);
	}

	getBus { | paramName |
		^subBusDict[paramName.asSymbol];
	}

	mapBus { | paramName |
		^subBusDict[paramName.asSymbol].asMap;
	}

	reMap {
		{
			this.prMapSynth;

			server.sync;

			{ viewElement[\multiSlider].valueAction_(currentValues.asArray); }.defer;
		}.forkIfNeeded;
	}

	liveMode { | active (true) |
		var enable;
		enable = active.not;
		{
			viewElement[\loadData].enabled = enable;
			viewElement[\loadMLP].enabled = enable;
		}.defer;
		live = active;
		^live;
	}

	autoTrainMLP { | cyclesNumber (1) |
		{
			cyclesNumber.asInteger.do({ arg i;
				mlp.fit(xydata, paramsdata, { arg loss;
					if ((i + 1) == cyclesNumber, {
						{ viewElement[\accuracy].string_(loss.round(0.00000001)); }.defer;
					});
				});
				0.05.wait;
			});
		}.forkIfNeeded;
	}

	xyMidiMap { | xCCNum, yCCNum, midiChan |
		var x, y;
		x = 0;
		y = 0;

		midiFunc !? ( _.free; );

		midiFunc = MIDIFunc.cc({ arg val, num, chan, src;
			var scaleValue, threshold, guiX, guiY;
			scaleValue = val.linlin(0, 127, 0, 1);
			if (num == xCCNum, {
				x = scaleValue;
			});

			if (num == yCCNum, {
				y = scaleValue;
			});

			threshold = 0.05;
			{
				guiX = viewElement[\xySlider].x;
				guiY = viewElement[\xySlider].y;

				if ( // Soft Takeover
					(active or: { (((x > (guiX - threshold)) and: { (x < (guiX + threshold)) }) and: { ((y > (guiY - threshold)) and: { (y < (guiY + threshold)) }) }) }),
					{
						xyModel[\x] = x;
						xyModel[\y] = y;
						active = true;
						xyModel.changed(\update);
					}
				);
			}.defer;
		}, [xCCNum, yCCNum], midiChan);

		format("% regressor x cc number: %, midi channel: %\n", defaultName, xCCNum, midiChan).post;
		format("% regressor y cc number: %, midi channel: %\n", defaultName, yCCNum, midiChan).post;

		^midiFunc;
	}

	setSynth { // for SynthDef
		var argValDict;
		argValDict = IdentityDictionary.new;

		paramArr.do({ arg param;
			argValDict[param] = subBusDict[param].getSynchronous;
		});

		if (isNdef, {
			synth.unmap(*paramArr);
		});

		synth.set(*argValDict.asKeyValuePairs);
	}

	// Private Methods

	prRejectParam { | controlNames |
		var cleanArr;

		cleanArr = controlNames.reject({ arg param;
			paramExclude.includesEqual(param.asString.toLower);
		});
		^cleanArr;
	}

	prInit {
		if (synth.isKindOf(Ndef), {
			var controlNames;
			if (synth.source.isNil, {
				^warn("Ndef function isNil");
			});
			isNdef = true;
			defaultName = defaultName ?? { synth.key; };
			defaultName = defaultName.asString;
			controlNames = synth.controlKeys;
			paramArr = this.prRejectParam(controlNames);
		}, {
			if (synth.isKindOf(Synth), {
				var synthName, controlNames;
				isNdef = false;
				synthName = synth.defName.asSymbol;
				defaultName = defaultName ?? { synthName; };
				defaultName = defaultName.asString;
				controlNames = SynthDescLib.global[synthName].controlNames;
				paramArr = this.prRejectParam(controlNames);
			}, {
				^warn("Synth or Ndef required");
			});
		});

		outputNumber = paramArr.size;

		subBusDict = IdentityDictionary.new;
		xyModel = IdentityDictionary.newFrom([\x, 0.0, \y, 0.0]);
		active = false;
		viewElement = IdentityDictionary.new;
		xydata = FluidDataSet(server);
		paramsdata = FluidDataSet(server);
		mlp = FluidMLPRegressor(
			server,
			[7],
			activation: FluidMLPRegressor.sigmoid,
			outputActivation: FluidMLPRegressor.sigmoid,
			maxIter: 1000,
			learnRate: 0.1,
			batchSize: 1,
			validation: 0
		);
		currentValues = Array.fill(outputNumber, {0.0});

		{
			controlBus = Bus.control(server, outputNumber);
			xybuf = Buffer.alloc(server, 2);
			paramsbuf = Buffer.alloc(server, outputNumber);

			server.sync;

			this.prCreateSynth;

			server.sync;

			this.prMapSynth;
			server.sync;

			this.prInitController;
			{
				this.prCreateView;
				this.liveMode(true);
				this.prCallBack;
			}.defer;
		}.forkIfNeeded;
	}

	prCallBack {
		action.value(this);
	}

	prInitController {
		controller = SimpleController(xyModel);
		controller.put(\update, { | theChanger, what, args |
			var x, y;
			x = theChanger[\x];
			y = theChanger[\y];
			xybuf.setn(0, [x, y]);
			regScaler.set(\t_trig_xy, 1);
			{
				viewElement[\xySlider].x = x;
				viewElement[\xySlider].y = y;
			}.defer;
		});
		controller.put(\set, { | theChanger, what, args |
			var x, y;
			x = theChanger[\x];
			y = theChanger[\y];
			xybuf.setn(0, [x, y]);
			regScaler.set(\t_trig_xy, 1);
		});
		^controller;
	}

	prMapSynth {
		paramArr.do({ arg param, i;
			var parameter, bus;
			parameter = param.asSymbol;
			subBusDict[parameter] = controlBus.subBus(i);

			// get current parameters values
			if (isNdef, {
				var currentVal;
				currentVal = synth.get(parameter);

				if (currentVal.isNumber, {
					subBusDict[parameter].set(currentVal);
					currentValues.put(i, this.prReverseScale(parameter, currentVal)); // set MultiSlider values
				});

				bus = subBusDict[parameter].asMap;
			}, {
				synth.get(parameter, { arg currentVal; // asynchronous
					if (currentVal.isNumber, {
						subBusDict[parameter].set(currentVal);
						currentValues.put(i, this.prReverseScale(parameter, currentVal));
					});
				});

				bus = subBusDict[parameter];
			});

			synth.map(parameter, bus);
		});
	}

	prUnMapSynth {
		var argValDict;
		argValDict = IdentityDictionary.new;

		paramArr.do({ arg param, i;
			var parameter;
			parameter = param.asSymbol;

			// get current parameters values
			if (isNdef, {
				var currentVal;
				currentVal = synth.get(parameter);

				case
				{ currentVal.isSymbol; } { argValDict[parameter] = subBusDict[param].getSynchronous; }
				// { currentVal.isSymbol; } { argValDict[parameter] = synth.getDefaultVal(parameter); }
				{ currentVal.isNumber; } {}
				{ currentVal.isKindOf(Bus); } { argValDict[parameter] = currentVal.getSynchronous; }
				{ argValDict[parameter] = synth.getDefaultVal(parameter); };
			}, {
				synth.get(parameter, { arg currentVal; // asynchronous
					if (currentVal.isNumber, {
						argValDict[parameter] = currentVal;
					});
				});
			});
		});

		if (argValDict.size > 0, {
			if (isNdef, {
				synth.unmap(*paramArr);
			});

			synth.set(*argValDict.asKeyValuePairs);
		});
	}

	prCleanUp {
		var synthDefName;
		synthDefName = regScaler.defName;

		synth.group !? { this.prUnMapSynth; };

		regScaler !? ( _.free; );

		controlBus !? ( _.free; );

		subBusDict.do({ arg bus;
			bus !? ( _.free; );
		});

		SynthDef.removeAt(synthDefName);

		controller !? ( _.remove; );

		mlp !? ( _.free; );
		xybuf !? ( _.free; );
		paramsbuf !? ( _.free; );

		oscFunc !? ( _.free; );
		midiFunc !? ( _.free; );

		xydata !? ( _.free; );
		paramsdata !? ( _.free; );
	}

	prReverseScale { |paramName, value|
		if (paramScale[paramName].notNil, {
			var scaleArr;
			scaleArr = paramScale[paramName];

			if (["exp", "exponential"].includesEqual(scaleArr[2].asString.toLower), {
				value = value.explin(scaleArr[0], scaleArr[1], 0, 1);
			}, {
				if (scaleArr[2].isNumber, {
					value = value.curvelin(scaleArr[0], scaleArr[1], 0, 1, scaleArr[2]);
				}, {
					value = value.linlin(scaleArr[0], scaleArr[1], 0, 1);
				});
			});
		});
		^value;
	}

	prScaleOutput { | outputArr |
		paramScale.keysValuesDo({ arg key, scaleArr;
			var index;
			index = paramArr.indexOf(key.asSymbol);

			if (index.notNil, {
				if (["exp", "exponential"].includesEqual(scaleArr[2].asString.toLower), {
					outputArr[index] = outputArr[index].linexp(0, 1, scaleArr[0], scaleArr[1]);
				}, {
					if (scaleArr[2].isNumber, {
						outputArr[index] = outputArr[index].lincurve(0, 1, scaleArr[0], scaleArr[1], scaleArr[2]);
					}, {
						outputArr[index] = outputArr[index].linlin(0, 1, scaleArr[0], scaleArr[1]);
					});
				});
			});
		});
		^outputArr;
	}

	prCreateSynth {
		{
			var scalerName, replyString;
			scalerName = ("regScaler" ++ UniqueID.next).asSymbol;
			replyString = "/" ++ scalerName.asString;
			SynthDef(scalerName, { arg bus, t_trig_xy = 0, gate = 1, predicting = 0;
				var values, trig, env/*, xy*/;

				env = EnvGen.kr(Env.asr, gate, doneAction: 2);

				trig = t_trig_xy * predicting;
				/*xy = FluidBufToKr.kr(xybuf);
				trig = Mix(Changed.kr(xy)) * predicting;*/
				mlp.kr(trig, xybuf, paramsbuf);
				values = FluidBufToKr.kr(paramsbuf);
				SendReply.kr(trig, replyString, values);

				values = this.prScaleOutput(values);
				Out.kr(bus, values);
			}).add;

			server.sync;

			regScaler = Synth(scalerName, [\bus, controlBus.index, \predicting, 0], group);

			oscFunc = OSCFunc({ arg msg;
				{ viewElement[\multiSlider].value_(msg[3..]); }.defer;
			}, replyString);
		}.forkIfNeeded;
	}

	prAddPoint {
		{ // allow to keep adding points after loading data
			var ids, id, cond;
			id = "point-%".format(UniqueID.next);
			cond = Condition(false);
			xydata.dump({ arg dict;
				ids = dict.keys;
				cond.test = true;
				cond.signal;
			}); // asynchronous

			cond.wait;

			while { ids.includes(id) } { id = "point-%".format(UniqueID.next) };

			xydata.addPoint(id, xybuf);
			paramsdata.addPoint(id, paramsbuf);
		}.forkIfNeeded;
	}

	prLoadDataDefault {
		var path, xyfile, paramsfile, filesExists;
		path = defaultPath ++ defaultName;
		xyfile = path ++ "-xydata.json";
		paramsfile = path ++ "-paramsdata.json";
		filesExists = (File.exists(xyfile) and: { File.exists(paramsfile) });

		if (filesExists, {
			xydata.read(xyfile);
			paramsdata.read(paramsfile);
		});

		^filesExists;
	}

	prLoadMlpDefault {
		var path, mlpfile, fileExists;
		path = defaultPath ++ defaultName;
		mlpfile = path ++ "-mlp.json";
		fileExists = File.exists(mlpfile);

		if (fileExists, {
			mlp.read(mlpfile);
		});

		^fileExists;
	}

	prCreateView {
		window = Window(defaultName, Rect(10, outputNumber, 840, 320));
		window.background = bgColor;
		window.onClose = { this.prCleanUp; };

		viewElement[\paramName] = StaticText().string_("parameter name").maxHeight_(15);
		viewElement[\paramValue] = StaticText().string_("value").maxHeight_(15);

		viewElement[\multiSlider] = MultiSliderView()
		.size_(outputNumber)
		.elasticMode_(1)
		.isFilled_(1)
		.action_({ arg ms;
			var sliderValues, index, scaleValues;
			sliderValues = ms.value;
			index = ms.index;

			paramsbuf.setn(0, sliderValues);

			scaleValues = this.prScaleOutput(sliderValues);

			{
				viewElement[\paramName].string_(paramArr[index]);
				viewElement[\paramValue].string_(scaleValues[index].round(0.0001));
			}.defer;
		})
		.valueAction_(currentValues.asArray);

		viewElement[\xySlider] = Slider2D()
		.action_({ arg view;
			xyModel[\x] = view.x;
			xyModel[\y] = view.y;
			xyModel.changed(\set);
			active = false;
		})
		.setXYActive(xyModel[\x], xyModel[\y]);

		if ((xCCNum.isInteger and: { yCCNum.isInteger }), {
			this.xyMidiMap(xCCNum, yCCNum, midiChan);
		});

		viewElement[\addData] = Button()
		.states_([["Add Data"]])
		.action_{
			this.prAddPoint;
		};

		viewElement[\clearData] = Button()
		.states_([["Clear Data"]])
		.action_{
			xydata.clear;
			paramsdata.clear;
		};

		viewElement[\saveData] = Button()
		.states_([["Save data"]])
		.action_{
			if (live.not, {
				Dialog.savePanel({ arg path;
					xydata.write(path ++ "-xydata.json");
					paramsdata.write(path ++ "-paramsdata.json");
				}, path: defaultPath);
			}, {
				var name;
				name = defaultPath ++ defaultName;
				xydata.write(name ++ "-xydata.json");
				paramsdata.write(name ++ "-paramsdata.json");
			});
		};

		viewElement[\loadData] = Button()
		.states_([["Load Data"]])
		.action_{
			Dialog.openPanel({ arg paths;
				var xypath, paramspath;
				xypath = paths.select({ arg path; path.contains("xydata"); });
				paramspath = paths.select({ arg path; path.contains("paramsdata"); });

				if(xypath.notNil, {
					xydata.read(xypath[0]);
				});

				if(paramspath.notNil, {
					paramsdata.read(paramspath[0]);
				});
			}, multipleSelection: true, path: defaultPath);
		};

		this.prLoadDataDefault;

		viewElement[\trainMLP] = Button()
		.states_([["Train MLP"]])
		.action_{
			mlp.fit(xydata, paramsdata, { arg loss;
				{ viewElement[\accuracy].string_(loss.round(0.00000001)); }.defer;
			});
		};

		viewElement[\accuracy] = StaticText().string_("Accuracy").maxHeight_(15);

		viewElement[\clearMLP] = Button()
		.states_([["Clear MLP"]])
		.action_{
			mlp.clear;
			{ viewElement[\accuracy].string_("Accuracy"); }.defer;
		};

		viewElement[\saveMLP] = Button()
		.states_([["Save MLP"]])
		.action_{
			if (live.not, {
				Dialog.savePanel({ arg path;
					mlp.write(path ++ "-mlp.json");
				}, path: defaultPath);
			}, {
				mlp.write(defaultPath ++ defaultName ++ "-mlp.json");
			});
		};

		viewElement[\loadMLP] = Button()
		.states_([["Load MLP"]])
		.action_{
			Dialog.openPanel({ arg path;
				mlp.read(path);
			}, path: defaultPath);
		};

		this.prLoadMlpDefault;

		viewElement[\prediction] = Button()
		.states_([["Not Predicting"], ["Predicting"]])
		.action_{ arg but;
			regScaler.set(\predicting, but.value);
		};

		window.layout = HLayout(
			VLayout(
				HLayout(
					viewElement[\paramName],
					viewElement[\paramValue]
				),
				viewElement[\multiSlider]
			),
			viewElement[\xySlider],
			VLayout(
				viewElement[\addData],
				viewElement[\clearData],
				viewElement[\saveData],
				viewElement[\loadData],
				viewElement[\trainMLP],
				viewElement[\accuracy],
				viewElement[\clearMLP],
				viewElement[\saveMLP],
				viewElement[\loadMLP],
				viewElement[\prediction]
			)
		);
		window.front;
		^window;
	}

}

I haven’t been able to thoroughly test it yet after making these changes

Another option would be two constructors, newSync, and newAsync. If you try calling the sync one on the main thread, it fails.

The reason why I think this is a problem is because it is really confusing when how you evaluate code affects what the code does. I’m taking about evaluating it line by line and wrapping it in brackets and evaluating it all at once.

Here one place this always comes up.

SynthDef(...);
Synth(...);

Since you asked for feedback on the code…

Another thing you might consider is moving all the logic from prInit into *new. Init methods have a number of problems, first you have made an object that hasn’t been initialised, it’s in an invalid state. This means you shouldn’t call any other methods inside the init method. Second, if someone (a users) inherits from this class and makes their own prInit method, your method will be overridden and you will never be able to get a valid object.

I thought the object existed (even if initialization wasn’t completely finished yet) after calling ^super.newCopyArgs(args…) and that its methods therefore also existed at that point.

That’s why I wouldn’t have thought it was a problem to call methods after that point (I call the methods in prInit, which is after newCopyArgs).

What particularly confuses me is that I can easily find plenty of widely shared quarks that do this (calling internal methods within the init method).

Example in SuperDirt:

SuperDirt {

	var <numChannels, <server;
	var <soundLibrary, <vowels;
	var <>orbits;
	var <>modules;
	var <>audioRoutingBusses;
	var <>controlBusses;
	var <group;
	var <flotsam;

	var <port, <senderAddr, <replyAddr, netResponders;
	var <>receiveAction, <>warnOutOfOrbit = true, <>maxLatency = 42;
	var <>dropWhen = false;
	var <>numRoutingBusses = 16, <>numControlBusses = 128;

	classvar <>default, <>maxSampleNumChannels = 2, <>postBadValues = false;

	*new { |numChannels = 2, server|
		^super.newCopyArgs(numChannels, server ? Server.default).init
	}

	*resetEverything {
		"===========> stopping all servers and recompiling the class library.".postln;
		Server.killAll;
		thisProcess.recompile;
	}

	init {
		soundLibrary = DirtSoundLibrary(server, numChannels);
		modules = [];
		this.checkServerMemory(50 * 1024);
		this.loadSynthDefs;
		this.initVowels(\counterTenor);
		this.initRoutingBusses;
		group = server.nextPermNodeID;
		flotsam = IdentityDictionary.new;
	}

I’d love to be able to implement what you’re telling me and adopt best practices, but unfortunately, I don’t see how to do that in a case like this.

If I understand correctly, using quarks is dangerous because their implementation often doesn’t follow best practices.

This is understandable given how difficult it is to find informations on what constitutes best practices.

I apologize if I wasn’t paying close enough attention and if these informations are already in the documentation.
Is there a place where I can find these informations ?

“Dangerous” in context of this thread is an unnecessary scare word.

What I was trying to point out in my last post is that reading a sound file into a buffer is dangerous, according to the definition floated out here: Buffer.read is a constructor, and at the time that the constructor returns, the buffer object is not ready for use (and it does exhibit the other problem that Jordan pointed out which is that code may behave differently when executed step by step versus all as a block). If one were to apply the standard proposed in this thread directly to buffer reading, then one might end up saying that sound files are dangerous. Reductio ad absurdum applies here, I think.

Again, I’ll agree that SC’s handling of asynchronous operations isn’t as slick as modern approaches, and I’m not saying there’s nothing to improve. But, we can put more emphasis on helping each other work toward at least reasonably stable behavior (edit: stable behavior in the code).

hjh

Yes the methods are there and you can do this, everything is allocated.

What particularly confuses me is that I can easily find plenty of widely shared quarks that do this (calling internal methods within the init method).

Yes, a lot has changed in the past 20 odd years since sc was made. If you look at modern OOP languages (like Dart) they make this behaviour much harder if not impossible, Rust also does something similar (but it doesn’t have inheritance so it is easier for them).

Consider a class that has two boolean, both must be a different value as this is what makes sense for this particular object (a silly example I know…)

C {
   var b1, b2;
   *new { |b| ^super.newCopyArgs(b.asBoolean).init }
    init {
       this.foo;
       ... assign b2 ...
    }
   foo { ... here what state is b1 and b2 in? ...}
}

The point is, when you write foo you assume that b1 and b2 have some particular state (being different) because that is what makes sense for the object… but if we call it inside the constructor, then object has not finished being initialised, and therefore we have no idea what state it is in (in this case it is nil but this is a small example). This means you can’t make assumption about the state of the object when writing methods because any of them might be called while the object is in the process of being initialised — ouch, we always do this when writing code (imagine having to check the type of every variable before you used it!).

One way to still have convenience methods is to use a ‘private’ static method, like this…

C {
   var b1, b2;
   *new { |b| ^super.newCopyArgs(b.asBoolean).init }
    init {
       C.prFooImpl(b1);
       ... assign b2 ...
    }
   *prFooImpl { |someBoolean| ... }
   foo { C.prFooImpl(b1) }
}

This is because the objects passed to prFooImpl have all been initialised (passing this would defeat the purpose).

I’d love to be able to implement what you’re telling me and adopt best practices, but unfortunately, I don’t see how to do that in a case like this.

Yes, that is often the case once the code is already written. Sometimes it isn’t worth the effort. I’m not saying you must do this. I’m just saying this is something to be aware of. Sometimes the best you can reasonably do is document something.

@jamshark70

When writing code for yourself I agree with your point (my personal code is full of things that only work in certain situations and is generally a functional mess), but when writing code for others to use, it is good to consider all the possible ways a user might attempt to use the code. So, yes, there are many places where using Buffer.read will produce weird results, potentially performance ending (aka ‘dangerous’) — new users have to learn all of these places and avoid them or mitigate them (s.sync).

I suppose there is a difference the standard the class library should hold itself to and code we share amongst ourselves, but still mitigating these weird edges cases should be a goal.

Absolutely!

I guess a car might serve as an analogy. Currently a car maker can’t stop you from driving into a tree (though self-driving car research might change that). Driving a car is dangerous but also brings conveniences that outweigh the risks, for most people (and acknowledging that in urban areas, public transit is generally a better investment etc etc). We don’t stop people from driving cars just because cars can be used incorrectly (edit: but we do try to make cars safer than they would be under a completely unregulated business environment).

Similarly – and while fully agreeing that public code should try to handle as many foreseeable cases as possible – I don’t think we can actually plug all the holes. There’s always going to be some way that some user somewhere can break something. At least in SuperCollider, the worst physical risk is loud loud noise in your headphones (vs a car, where the wrong accident can cost lives).

100% agree that it’s good to discuss how to improve stability. At the same time, I got a sense that kesey was feeling some anxiety about having written something “dangerous” – there might be other types of language that get the same educational job done without the added emotional layer. E.g., “There’s a potential problem here… what if the user creates your object and then immediately calls x?” and then move toward solutions.

hjh

1 Like

Thank you for the tips and advices.
I’m really sorry for using the word “dangerous”.
Shame on me