ART* should match IDL definition

[ricardopereira]

As agreed on the call, we will update the ART* classes to match the IDL. The ones we know need changing are below, but @ricardopereira please review all classes before submitting a PR for this issue:

• ARTRealtimeConnectionState -> ARTConnectionState
• ARTRealtimeConnectionEvent -> ARTConnectionEvent
• ARTRealtimeChannelState -> ARTChannelState
• ARTRealtimeChannelEvent -> ARTChannelEvent

The IDL is at http://docs.ably.io/client-lib-development-guide/features/#idl

[mattheworiordan]

@ricardopereira @tcard further to the conversation at #543 (review), I have given this some thought, and I am in fact now against the change. My thinking is:

• It's an unnecessary breaking change
• It means we have to update all tutorials and documentation, this is a complex task for tutorials as each tutorial's commits need to be updated and then each tutorial needs updating, see https://github.com/ably/tutorials/pulls and Changed to match current library distribution status docs#213 (review) for reasons why this is hard.
• We have a ARTChannel and ARTRealtimeChannel which reflects the IDL which has Channel and RealtimeChannel. We may later have ARTConnection for REST (although unlikely), so if we did having ARTRealtimeConnection is not such a bad thing.
• If we have ARTChannel and ARTRealtimeChannel, then when considering ARTRealtimeChannel* to ARTChannelState* #556, ARTChannelState should indeed stay as ARTRealtimeChannelState`.

In short whilst I think the naming is not perfect, changing it does not really achieve much, but will cause pain for customers and us in regards to documentation, examples we have, tutorials etc.

Is there really any compelling reason to change this now?

FYI @paddybyers @SimonWoolf

[ricardopereira]

@mattheworiordan Ok, I agree with most of it. We should at least rename each cases from ARTRealtimeConnectionState:

typedef NS_ENUM(NSUInteger, ARTRealtimeConnectionState) {
    ARTRealtimeInitialized, //<---- ARTRealtimeInitialized to ARTRealtimeConnectionInitialized
    ARTRealtimeConnecting,
    ARTRealtimeConnected,
    ARTRealtimeDisconnected,
    ARTRealtimeSuspended,
    ARTRealtimeClosing,
    ARTRealtimeClosed,
    ARTRealtimeFailed
};

[ricardopereira]

Well, maybe it's not so important because in Swift, it gets translated to ARTRealtimeConnectionState.Initialized so it will only affect Objective-C code. Could be something to be done after the v0.9.

[ricardopereira]

BTW, @tcard I need some feedback from you.

From IDL de ConnectionEvent embeds the ConnectionState:

enum ConnectionState:
  INITIALIZED
  CONNECTING
  CONNECTED
  DISCONNECTED
  SUSPENDED
  CLOSING
  CLOSED
  FAILED

enum ConnectionEvent:
  embeds ConnectionState
  UPDATE // RTL4h

Which change seems more correct to you?

1. Duplicate the ARTRealtimeConnectionState as ARTRealtimeConnectionEvent and add the new UPDATE case
2. Use the last ARTRealtimeConnectionState and increment it on the ARTRealtimeConnectionEvent:

typedef NS_ENUM(NSUInteger, ARTRealtimeConnectionState) {
    ARTRealtimeInitialized,
    ARTRealtimeConnecting,
    ARTRealtimeConnected,
    ARTRealtimeDisconnected,
    ARTRealtimeSuspended,
    ARTRealtimeClosing,
    ARTRealtimeClosed,
    ARTRealtimeFailed
};

typedef NS_ENUM(NSUInteger, ARTRealtimeConnectionEvent) {
    ARTRealtimeUpdate = ARTRealtimeFailed + 1
};

This will be translated to Swift like:

public enum ARTRealtimeConnectionState : UInt {
    case Initialized
    case Connecting
    case Connected
    case Disconnected
    case Suspended
    case Closing
    case Closed
    case Failed
}

public enum ARTRealtimeConnectionEvent : UInt {
    case Update
}

[tcard]

The first case. The second case is not correct at all. ARTRealtimeConnectionEvent must have all those variants, not only Update.

I'm not opposed to dropping the name changes, by the way. I'd rather have them match the IDL (ie. ConnectionState and ChannelState), but well.

[mattheworiordan]

@tcard @ricardopereira can we perhaps discuss this tomorrow as it's important we get this right in 0.9 and minimise breaking changes at the same time.

[ricardopereira]

@tcard @mattheworiordan By implementing the UPDATE event, I noticed that it will always exist a breaking change (at least for ObjC) because the Emitter will change (it will emit an ARTRealtimeConnectionEvent instead of an ARTRealtimeConnectionState), so code like:

[self.connection once:ARTRealtimeClosed callback:^(ARTConnectionStateChange *stateChange) {
    [expectation fulfill];
}];

should be updated to

[self.connection once:ARTRealtimeConnectionEventClosed callback:^(ARTConnectionStateChange *stateChange) {
    [expectation fulfill];
}];