These are chat archives for ericelliott/stampit

3rd
Jun 2015
Vasyl Boroviak
@koresar
Jun 03 2015 00:34

Let me reimplement the code above using C#:

class Person {
  public string name { get; set; }
  public string getName() { return this.name; }
}
class LoudPerson {
  public override string getName() { return base.getName() + "!"; } // This is a code smell!
}

In C# the base is similar to the Java super. By using these things you tightly couple LoadPerson to Person.

Carl Olsen
@unstoppablecarl
Jun 03 2015 00:35
I understand that. I am asking what are the alternatives to using existing function code and adding to it?
Vasyl Boroviak
@koresar
Jun 03 2015 00:35
I'd recommend avoiding that by the has-a principle. https://github.com/ericelliott/stampit/issues/90#issuecomment-107774165
Let me show an alternative. 1 min
Carl Olsen
@unstoppablecarl
Jun 03 2015 00:36
I don’t think the stampit example is completely equivalent
it is close
Vasyl Boroviak
@koresar
Jun 03 2015 00:36
sure
But still you tightly coupled LoudPerson to Person.
Carl Olsen
@unstoppablecarl
Jun 03 2015 00:37
loudPerson has a dependency, yes. it assumes it is being composed with a stamp that has a method ‘getName'
Vasyl Boroviak
@koresar
Jun 03 2015 00:37
Exactly.
Carl Olsen
@unstoppablecarl
Jun 03 2015 00:38
is that really “tightly coupled” ?
Vasyl Boroviak
@koresar
Jun 03 2015 00:38
Yes.
Carl Olsen
@unstoppablecarl
Jun 03 2015 00:38
this is a trivial example so it lacks context. but how else would you do it?
there will be other code somewhere assuming the object has a method getName
why is the composition of the stamp different?
Vasyl Boroviak
@koresar
Jun 03 2015 00:40
var Nameable = stampit()
    .props({
        name: null
    })
    .methods({
        getName: function(){
            return this.name;
        }
    });
var LoudNamable = Nameable
    .methods({
        getName: function(){
            return this.name + '!';
        }
    });

var Person = stampit({ ... }).compose(Nameable);
var LoudPerson = Person.compose(LoadNameable);
oops!!! too early..
wait a minute
Carl Olsen
@unstoppablecarl
Jun 03 2015 00:43
I know this is a trivial example what if there was a fair amount of crunching going on in that method?
Vasyl Boroviak
@koresar
Jun 03 2015 00:43
Objects should be composed from their abilities.
Carl Olsen
@unstoppablecarl
Jun 03 2015 00:43
that it would not make sense to copy the code?
Vasyl Boroviak
@koresar
Jun 03 2015 00:44
In fact. I would never name the function getName same way. New logic deserves its own function. Like getLoudName.
This would make code more explicit.
Carl Olsen
@unstoppablecarl
Jun 03 2015 00:45
That would be ideal. but if you are working with an existing codebase with lots of places using this getName function that you want to use the extended method?
I agree that this is not ideal but I think there are valid cases for it
Vasyl Boroviak
@koresar
Jun 03 2015 00:46
I see.
Carl Olsen
@unstoppablecarl
Jun 03 2015 00:47
I’m not suggesting you go for the class extension pattern from the start but there are cases this is valid I think
Vasyl Boroviak
@koresar
Jun 03 2015 00:47
Agree.
Carl Olsen
@unstoppablecarl
Jun 03 2015 00:48
thanks for taking time to explain
I noticed something in the read me that I wanted to get your thoughts on
Vasyl Boroviak
@koresar
Jun 03 2015 00:49
In this case your approach is legit. Although, if we put that to the advanced_examples.md then we have to put many exclamations marks and note that we discourage poeple to do such a trick.
Carl Olsen
@unstoppablecarl
Jun 03 2015 00:50
you are probably correct about that
the readme doesn’t explain this anywhere and I think it may be unexpected to some people. What do you think?
var person = stampit();

// makes new stamp
person.init(function(){});
person = person.init(function(){});
the same for any of the chain methods
I actually caught myself making this mistake at first
Vasyl Boroviak
@koresar
Jun 03 2015 00:52
Actually it does explain that in multiple places.
Carl Olsen
@unstoppablecarl
Jun 03 2015 00:52
I only saw it metioned in the api section at the end
Vasyl Boroviak
@koresar
Jun 03 2015 00:52
But you can add more warnings to the README where needed.
Carl Olsen
@unstoppablecarl
Jun 03 2015 00:54
I just skimmed it and didn’t see anything before the api part
Feel free to edit README the way you see it mate :)
Carl Olsen
@unstoppablecarl
Jun 03 2015 00:54
OHHHH it mutated in v 1.x ?
Vasyl Boroviak
@koresar
Jun 03 2015 00:55
Exactly.
Carl Olsen
@unstoppablecarl
Jun 03 2015 00:55
there are more places than I realized
I think there might be too much in the read me. what do you think
Vasyl Boroviak
@koresar
Jun 03 2015 00:56
All chaining methods return new stamps instead of self-mutating this stamp.
and
Chaining stamps always creates new stamps.
Carl Olsen
@unstoppablecarl
Jun 03 2015 00:56
?
then again there is a lot to explain and document that is not simple
Vasyl Boroviak
@koresar
Jun 03 2015 00:57
Readme must be as clear as possible. Actually, it's soooo large. Thus people don't see the most important parts of it.
Carl Olsen
@unstoppablecarl
Jun 03 2015 00:58
I think there are some things in it that are too general for the primary readme
like the more chaining part?
I think everything from the start to the what is a stamp section is good after that I think it kinda gets lost
what do you think our must haves are after that point in the readme?
Vasyl Boroviak
@koresar
Jun 03 2015 01:04
I don't know, my friend. Realistically, I can't judge it's quality anymore. I read and edited it thousands of times.
Please, propose your ideas in a GitHub issue. We'd discuss there.
Carl Olsen
@unstoppablecarl
Jun 03 2015 01:06
ok
need to think about it some more
Vasyl Boroviak
@koresar
Jun 03 2015 01:06
Or, better, draft it somehow (in a branch maybe?).
It's up to you. Thanks for the help anyway.
Carl Olsen
@unstoppablecarl
Jun 03 2015 01:06
np
Eric Elliott
@ericelliott
Jun 03 2015 19:52

Objects should be composed from their abilities.

YES.

Eric Elliott
@ericelliott
Jun 03 2015 19:57

That would be ideal. but if you are working with an existing codebase with lots of places using this getName function that you want to use the extended method?

It is very dangerous to alter the behavior of something that a lot of callers depend on. Instead of altering the behavior, you should deprecate the previous version and refactor callers to use the new version, otherwise, you could potentially break many dependents and not be aware that they're broken.

Since you're actually creating a new factory, you would have to refactor callers anyway to use the new thing, so you might as well create a more semantic name to describe the new behavior.
Changing anything about the behavior of a method is a breaking change. Robust software is open to extension, but closed to breaking changes (the open/closed principle).
Carl Olsen
@unstoppablecarl
Jun 03 2015 20:01
that is a good point
Eric Elliott
@ericelliott
Jun 03 2015 20:01
The open/closed principle is why there are things about the web standards that were considered bugs that we're now stuck with forever, because the specifications had to be closed to breaking changes and lots of websites depended on the buggy behavior.
Fixing the "bugs" now would break a lot of sites.
Eric Elliott
@ericelliott
Jun 03 2015 20:07
The new thing could still use the same name, even, because you're not replacing the old thing. The way you would do that is to make the new thing a wrapper around the old thing, and then implement the new methods by calling the public API of the other methods. That's not "call super" because it's not an "is-a" relationship anymore, it's a "has-a" relationship. The old thing is encapsulated inside the new thing instead of the new thing being an instance of the old thing.
"is-a" (classical / call-super) vs "has-a" or "uses-a" (composition)
subtle distinctions in theory, but huge software quality ramifications in practice.
Carl Olsen
@unstoppablecarl
Jun 03 2015 20:25
that makes a lot of sense thanks
Vasyl Boroviak
@koresar
Jun 03 2015 21:14
Well said Eric. As always.