tc39 / proposal-first-class-protocols

a proposal to bring protocol-based interfaces to ECMAScript users
352 stars 9 forks source link

Different syntax for required key? #18

Closed rauschma closed 7 years ago

rauschma commented 7 years ago

I’d prefer:

protocol ProtocolName requires thisMustBeImplemented {
}

Or:

protocol ProtocolName(thisMustBeImplemented) {
}
nicolo-ribaudo commented 7 years ago

I prefer the current proposal: the methods names become the keys of the protocol object, so having them inside the braces is more consistent with objects and classes:

Object.a; -> const Object = { a };

Class.a; -> class Class { static a; }

Protocol.a; -> protocol Protocol(a) {}; // There is {} but Protocol.a isn't defined there? 😕

I think there is also a stylistic argument: when there are multiple required methods split in different lines, they don't look very nice in the protocol header:


// Without parens, this is the best-looking formatting I could think of.
// Note the two level indentation to distinguish them from the "extends" part.
protocol ProtocolName requries
    thisMustBeImplemented1,
    thisMustBeImplemented2,
    thisMustBeImplemented3,
  extends SuperProtocol {
    youGetThisMethodForFree() {}
  }

// With parents it is a little better (it looks like function parameters)
protocol ProtocolName(
  thisMustBeImplemented1,
  thisMustBeImplemented2,
  thisMustBeImplemented3,
) extends SuperProtocol {
  youGetThisMethodForFree() {}
}

// This looks like a class or an object :D
protocol ProtocolName extends SuperProtocol {
  thisMustBeImplemented1;
  thisMustBeImplemented2;
  thisMustBeImplemented3;

  youGetThisMethodForFree() {}
}
``
michaelficarra commented 7 years ago

Couldn't have said any of it better myself. Thanks @nicolo-ribaudo, I completely agree.

rauschma commented 7 years ago

@nicolo-ribaudo Makes sense, thanks!

One alternative:

protocol ProtocolName extends SuperProtocol {
  require thisMustBeImplemented1;
  require thisMustBeImplemented2;
  require thisMustBeImplemented3;

  youGetThisMethodForFree() {}
}