[Refactor] Rocket now subclasses ComponentAssembly #293
Conversation
…mponent - conceptually, a rocket *IS A* ComponentAssembly. - reduces code duplication: 7 redundant functions - rocket still inherits all the RocketComponent methods - Also: changed Coordinate.NUL -> Coordinate.ZERO to better reflect our intent. -- Nitpick: Coordinate.NUL is *not* null. It's zero. These are different! -- Null is an error condition. -- Zero is a valid coordinate. Zero means something is located at the origin. Like the rocket, and the nosecone.
|
I not familiar with the code, but be careful of thinking "Is A" means inheretance, it doesn't. "Is A" means "is constructed as". Proper inheritance is a "behaves as" relationship. Craig Sent from TypeApp On Oct 13, 2016, 15:41, at 15:41, Daniel Williams [email protected] wrote:
|
|
that actually makes sense, cleans up a lot of stub methods |
|
now just gotta figure out why that value is now half :/ |
I use "is a" primarily because that's the most common way of phrasing
What do you mean? Obviously, I can define two different classes, with the On Thu, Oct 13, 2016 at 5:53 PM, SpamWall [email protected] wrote:
|
|
@Vicilu WHAT value? If you've found a bug in that code, please be more specific! All the core/unit tests pass, excepting IntegrationTest, which we already knew about. |
|
For instance, a square "Is A" rectangle, but a square does not "Behave As" a rectangle. A rectangle can change length and width independently. A square cannot. Having a Square class inherit from a Rectangle class means some of the method are not polymorphic, some methods just don't make any sense in the scope of the derived class, and "instance of" calls start creeping in. Sent from TypeApp On Oct 13, 2016, 19:14, at 19:14, Daniel Williams [email protected] wrote:
|
|
Yea, that makes sense. I was making the is-a statment in contrast to
has-a, instead of the more nuanced distinction between is-a and behaves-as.
Obviously, we agree that just because it COULD be defined by inheritance,
doesnt mean it SHOULD...
but to bring this back on topic, what's the best choice here? (PR#293)
|
|
oh yes, that integration test was what I was talking about |
|
and I think the best choice is to apply this change, it makes more sense and has less "dead code" |
|
As this package is the source of almost all problems with unstable, I've been documenting it and found a lot of mess, parent depending of child and other things, need a lot of refactoring, who knows? my guess is this is the first step to solve the problems |
4 participants
. . . instead of RocketComponent
Discussion:
Arguably this is a non-trivial change, but this simplifies overrides, specifically when debugging mass-calculation code, and locating components. My case is:
* 7 redundant functions
* lines: +8 / -47
[edited for format.]