Repository: NativeScript/NativeScript
File: packages/core/ui/layouts/layout-base-common.ts (TypeScript)

Code before:

	getChildIndex(child: View): number {
		return this._subViews.indexOf(child);
	}

	public getChildById(id: string) {
		return getViewById(this, id);
	}

	public _registerLayoutChild(child: View) {
		//Overridden
	}

	public _unregisterLayoutChild(child: View) {
		//Overridden
	}

	public addChild(child: View): void {
		// TODO: Do we need this method since we have the core logic in the View implementation?
		this._subViews.push(child);
		this._addView(child);
		this._registerLayoutChild(child);
	}

	public insertChild(child: View, atIndex: number): void {
		if (atIndex > -1) {
			this._subViews.splice(atIndex, 0, child);
			this._addView(child, atIndex);
			this._registerLayoutChild(child);
		}
	}

	public removeChild(child: View): void {
		this._removeView(child);

		// TODO: consider caching the index on the child.
		const index = this._subViews.indexOf(child);
		if (index > -1) {
			this._subViews.splice(index, 1);
			this._unregisterLayoutChild(child);
		}
	}

	public removeChildren(): void {
		while (this.getChildrenCount() !== 0) {
			this.removeChild(this._subViews[this.getChildrenCount() - 1]);
		}
	}

	get padding(): string | CoreTypes.LengthType {
		return this.style.padding;

Review comment (suggestion):
While this prevents the flow from proceeding, it does not prevent subclass overrides from executing their own logic.

These are the cases that need checking:
https://github.com/NativeScript/NativeScript/blob/main/packages/core/ui/layouts/layout-base.ios.ts#L14
https://github.com/NativeScript/NativeScript/blob/main/packages/core/ui/layouts/root-layout/index.android.ts#L11

We can either add the `atIndex > -1` check there too or change the return type of the method (let's say `boolean` just like `_addViewToNativeVisualTree`) and be able to perform checks the following instead:

```ts
if (super.insertChild(child, index)) {
    // Proceed
}
```

This approach might implicitly help plugin maintainers realize they'll have to check what the super call returns.

Code after:

	getChildIndex(child: View): number {
		return this._subViews.indexOf(child);
	}

	public getChildById(id: string) {
		return getViewById(this, id);
	}

	public _registerLayoutChild(child: View) {
		//Overridden
	}

	public _unregisterLayoutChild(child: View) {
		//Overridden
	}

	public addChild(child: View): void {
		// TODO: Do we need this method since we have the core logic in the View implementation?
		this._subViews.push(child);
		this._addView(child);
		this._registerLayoutChild(child);
	}

	public insertChild(child: View, atIndex: number): boolean {
		if (atIndex > -1) {
			this._subViews.splice(atIndex, 0, child);
			this._addView(child, atIndex);
			this._registerLayoutChild(child);
			return true;
		}
		return false;
	}

	public removeChild(child: View): void {
		this._removeView(child);

		// TODO: consider caching the index on the child.
		const index = this._subViews.indexOf(child);
		if (index > -1) {
			this._subViews.splice(index, 1);
			this._unregisterLayoutChild(child);
		}
	}

	public removeChildren(): void {
		while (this.getChildrenCount() !== 0) {
			this.removeChild(this._subViews[this.getChildrenCount() - 1]);
		}
	}