frostburn / ts-geometric-algebra

TypeScript Geometric Algebra Generator
MIT License
12 stars 2 forks source link

Suggestion for `this.cls()` #16

Closed kungfooman closed 8 months ago

kungfooman commented 2 years ago

I looked a bit through the code and wondered how class inheritance for proper OOP is implemented and I found this e.g.:

https://github.com/frostburn/ts-geometric-algebra/blob/bba5389f3ba0ef66295ce5b2b21592c343e4be55/src/pqr.ts#L87-L91

It looks like the only job of the cls method is to return this.constructor and if the base class had that method, you wouldn't need to override cls in every other class.

This is: 1) adding extra complexity for whoever wants to extend a class 2) adding bracket salad like new (this.cls())()

cls can just be a getter to remove the extra new (...()) and through inheritance it will always point to its own class:

interface ConstructorOf<T> {
  new(): T;
}
class Animal {
  get cls(): ConstructorOf<this> {
    return this.constructor as any;
  }
  clone() {  
    const ret = new this.cls;
    return ret;
  }
}
class Mammal extends Animal {
  // Nothing yet.
}
const animal1 = new Animal;
const animal2 = animal1.clone();
const mammal1 = new Mammal;
const mammal2 = mammal1.clone();

TS PlayGround: https://www.typescriptlang.org/play?#code/JYOwLgpgTgZghgYwgAgMIHsQGcxQK4JjpQDyMAPACoB8yA3gLABQyyIEA7gBQCUAXMkoBuZgF9mCADZwsWZAEEQwALZxJ9ZqwDmEMMilZeAjNlwEipCmAAWwLLUYtWyKLrxQQyG3YB0CTDj4hMTIMqEgAJ4iTuJOUpgQvPSsms7+pi66yAC8bJxetlh+kljRzplg7p6uYGXIsbFSMnIAsnDKquoQAB6QIAAmcooqahpOAPTjyABy6N4gWsgRuj5iEgF6cEqdAIw5eRwK22rR6TjhI5IATPtblzvFCbynG8iqHWp7ueyHbR+SLwy706N1ywM+j3Yz2YQA

I didn't test this library yet, but I still wonder what else is the point of the cls method?

frostburn commented 2 years ago

> I still wonder what else is the point of the cls method? If you look at the commit history you will see that all of the pqr.ts stuff used to be sprinkled about index.ts behind various if statements. The cls shenanigans get around TypeScipt's lack of abstract static methods. cls() is a lot cleaner than the previous if soup, but maybe not as clean as it could be...

So thank you for the tip! I'm still very new to this TypeScript stuff I will look into cleaner ways to implement the overrides.

frostburn commented 2 years ago

Unfortunately I can't make your interface trick work with a more complex toy example. (I'll try turning cls into a getter though.)

interface ConstructorOf<T> {
  new(): T;
}
class Animal {
  age: number
  constructor(age: number ) {
    this.age = age;
  }
  get cls(): ConstructorOf<this> {
    return this.constructor as any;
  }
  clone() {  
    const ret = new this.cls(this.age);  // Doesn't know about what the number of arguments should be
    return ret;
  }
  breed() {
    return this.cls.baby();  // Doesn't know that there's a static method
  }

  static baby() {
    return new Animal(0);
  }
}

TS PlayGround: https://www.typescriptlang.org/play?#code/JYOwLgpgTgZghgYwgAgMIHsQGcxQK4JjpQDyMAPACoB8yA3gLABQyyIEA7gBQCUAXMkoBuZgF9mCADZwsWZAEEQwALZxJ9ZqzgBzCAJB5lAI2ibkCTDnyFiXHXraGTUZDw0tWyMAAtgWAHT2yAC8yPYiHuIeumDmkli8AhjYuAREpBQ+frSMHqxQEGB4UCBevgEWKdbpYXJwIACeEaxRrFKYELz0rGZtlrEFsaHsHGV+-lIJWQH2PEKsAPQLyAAi6BBYIADksQDWIOijcEboeLEc3nCxPigGxtDI6DBhUNqGEOByWN6nkgAmyBMvWQg2KpUGzWQrUBBQgfy6uU8IMKYLGFXi-iMxwavHmyCWq3Wmx2yH2hzKVzK0AgWzqyBwV2ACGQykKPz+ZmhZgZYCZgOxCOBoJKbE4CiUqkkXAADHNOWJmEA

kungfooman commented 2 years ago

Thank you for implementing the getter suggestion right away, for the rest of the issues there are also solutions:

  age: number
  constructor(age: number ) {
    this.age = age;
  }

Just a note, you can shortcut this into:

  constructor(public age: number ) {
    // Nothing yet.
  }

This is hard-coded into Animal, which causes problems for extending the class:

  static baby() {
    return new Animal(0);
  }

To make it work properly in extending classes, this should be:

  static baby() {
    return new this(0);
  }

To fix the static baby error, some type programming needs to be done:

console.clear();
export type AnimalConstructor<C> = {
  new (...args: ConstructorParameters<typeof Animal>): C;
} & {
  [Q in keyof typeof Animal]: typeof Animal[Q]
};
class Animal {
  constructor(public age: number) {
    // Nothing yet.
  }
  get cls(): AnimalConstructor<this> {
    return this.constructor as any;
  }
  clone() {  
    const ret = new this.cls(this.age);
    return ret;
  }
  breed() {
    return this.cls.baby();
  }
  static baby() {
    return new this(0);
  }
}
class Mammal extends Animal {
  // Nothing yet.
}
const unittests = [
  () => new Animal(10).age == 10,
  () => new Mammal(20).age == 20,
  () => new Animal(10).clone().age == 10,
  () => new Mammal(20).clone().age == 20,
  () => new Animal(10).clone() instanceof Animal,
  () => new Mammal(20).clone() instanceof Mammal,
  () => Animal.baby() instanceof Animal,
  () => Mammal.baby() instanceof Mammal,
];
console.table(
  unittests.map((_,i) => ({
    test: _.toString().slice(6),
    result: _()
  }))
);

TS PlayGround: https://www.typescriptlang.org/play?#code/MYewdgziA2CmB0w4EMBOAKAlAbgLAChYAPABxFQBcACCgTxNioEEwBLAW2WgGFwILUAV2AVyAHm4A+KgF4qAbwJUqYWAHcq6eNrQBzCAC4qvSAOGjUABTTJ2sCrFQQxdBiABmzNp2iTMR7jx8AF8qADIFJSoAbQBFKlYwKgBrWFoPGnpYDJYOLgBdI1dsz1yfOPyCYKCkZAgILzzoSPxlUFMhEXJ0EkEAI2hWYCpkXVgjMEF2PsdMFuVlAHpFqgA5EAoAC0TdKlp7eCjgqLHqJAgsIzKuE35OixdtiGlFVoXUe0FUJK3WCEQ+GYuqgRg1kGBaEFlMc3khwLAsAplFE2oCqB9qHJVBpfv9zuhcfBRrAcCj0Z9vuSKFCqDDlH0PrAACaI14LKlfH5PRDQf59ZB9WhYGl0qj8ZAUIZUfmC1lkjGclTqGhPdAABlJbxhMNq9SoAFlbD4qMQHGAmQ1rs02VRlmsNtswLt9hRDiECO1+FRBGwKA5+A05NEooiZNJsY0fOgAIwaoljWRyWMAGhDczDSo0hvYUYATHHiYmqPnU29Q+HlVaY3G4aosPHGDIk2rS8py5mDUauOh85gefD64Wm8WW2nZBWNFXY33awi5olxWBgCVI1xW5p0xPOznu73+3X56ZwcuMtmfOv21b4DKhYfFyfSt412OM2euNeBbeEkelyu39BS3yGo+BgBAKAFOB0CiH1WD9WAA3gTgSHQdAAH1k1YTdNBtZR-QoIxUPgUQAGUBB2esIEGZd0AANkwddlA+CBBGgfCqFQrAjkwTACE1IA

The generic type is not exactly "nice and easy" anymore though and this doesn't work the way I would wish it to, the generic type needs to fall back onto typeof Animal all the time 😞

frostburn commented 8 months ago

Made a little bit less hacky but still hacky.

kungfooman commented 8 months ago

Too bad TS can't even infer the type of this.constructor, but n1