Do we get how getters should work?

[jodator]

Provide a description of the task

So we have some nice set of rules for using getters. But it seems that we've forgot about that and most of the editor engine objects do throw:

ckeditor5/packages/ckeditor5-engine/src/view/node.js

Lines 62 to 80 in 71d8192

	get index() {
	let pos;
	if ( !this.parent ) {
	return null;
	}
	// No parent or child doesn't exist in parent's children.
	if ( ( pos = this.parent.getChildIndex( this ) ) == -1 ) {
	/**
	* The node's parent does not contain this node. It means that the document tree is corrupted.
	*
	* @error view-node-not-found-in-parent
	*/
	throw new CKEditorError( 'view-node-not-found-in-parent: The node\'s parent does not contain this node.', this );
	}
	return pos;
	}

ckeditor5/packages/ckeditor5-engine/src/model/markercollection.js

Lines 407 to 413 in aac7ebd

	get affectsData() {
	if ( !this._liveRange ) {
	throw new CKEditorError( 'marker-destroyed: Cannot use a destroyed marker instance.', this );
	}
	return this._affectsData;
	}

I recall that those can cause troubles in integrators code. For instance I've tried to use Markers inside functional React components. For sure I lacked a how-to knowledge on how to use non-functional objects in a functional world. But this might showcase potential problems in our code in terms of design, maintainability and ease of integration for outside apps.

I can see why we've introduced some errors checking here and there. Also, our code base tries to follow object-oriented paradigm so patterns of functional programming might seems as not so relevant. But even so, we should avoid nasty side-effects. Also code that throws is harder to use.

[Reinmar]

The story that I remember is that we started abusing getters so we created rules when not to use them. And then we didn't abuse them so we stopped caring about those rules that much.

As for whether a getter may throw... I'd say it should not as it seems unintuitive to get an error when you access a property. But then, what should pos.index be if the offset is incorrect? -1? null? Both would be fine but an error gives us a feedback faster and that feedback is easier to understand.

Therefore, I have no strong opinion here. Perhaps both these getters here should not be getters. Or we should allow exceptions in cases where throwing an error may save people grey hair.

[CKEditorBot]

There has been no activity on this issue for the past year. We've marked it as stale and will close it in 30 days. We understand it may still be relevant, so if you're interested in the solution, leave a comment or reaction under this issue.

[CKEditorBot]

We've closed your issue due to inactivity over the last year. We understand that the issue may still be relevant. If so, feel free to open a new one (and link this issue to it).