Skip to content
This repository has been archived by the owner on Nov 27, 2023. It is now read-only.

Refactor concerns in Service Bus *Info and *Description classes #469

Closed
jcookems opened this issue Jun 12, 2012 · 6 comments
Closed

Refactor concerns in Service Bus *Info and *Description classes #469

jcookems opened this issue Jun 12, 2012 · 6 comments

Comments

@jcookems
Copy link
Contributor

Currently, in the Java SDK, the TopicInfo class extends EntryModel<TopicDescription>, and has these public properties:

Entry (inherited from EntryModel)
Model (inherited from EntryModel)
Path                                 maps to Entry
DefaultMessageTimeToLive             maps to Model
DuplicateDetectionHistoryTimeWindow  maps to Model
EnableBatchedOperations              maps to Model
MaxSizeInMegabytes                   maps to Model
RequiresDuplicateDetection           maps to Model
SizeInBytes                          maps to Model

It is the best of both worlds, the properties we are interested in are exposed at the top level, and the only implementation details are the Entry and Model properties, which encapsulate the junk needed elsewhere. The Has-A relationship plus the facade pattern works well here.

The same class in PHP is not as clean, because the TopicInfo extends Entry, so all its implementation details are top-level, while the stuff the user cares about (the model parts above) must be accessed through the TopicDescription property.

Here are the properties of the PHP SDK TopicInfo class, which extends Entry:

Attributes       (inherited from Entry)
Author           (inherited from Entry)
Category         (inherited from Entry)
Content          (inherited from Entry)
Contributor      (inherited from Entry)
ExtensionElement (inherited from Entry)
Id               (inherited from Entry)
Link             (inherited from Entry)
Published        (inherited from Entry)
Rights           (inherited from Entry)
Source           (inherited from Entry)
Summary          (inherited from Entry)
Title            (inherited from Entry)
Updated          (inherited from Entry)
TopicDescription

The TopicDescription property returns an instance of the TopicDescription class, which has these properties:

DefaultMessageTimeToLive
DuplicateDetectionHistoryTimeWindow
EnableBatchedOperations
MaxSizeInMegabytes
RequiresDuplicateDetection
SizeInBytes

The Is-A exposes lots of implementation details that the user should never care about (and most are not even applicable for SB service), and the lack of facade means what the user does care about is hidden in one out of 16 properties.

@ghost ghost assigned antonba Jun 12, 2012
@ghost
Copy link

ghost commented Jun 15, 2012

Agreed this is a design issue that adds complexity if we don't fix it. Would be good to get people on the same page for a design that is crisper.

@antonba
Copy link
Contributor

antonba commented Jun 15, 2012

We agreed to fix this formerly P2 issue as well due to the breaking changes it introduces to the developer experience.

@antonba
Copy link
Contributor

antonba commented Jun 20, 2012

So Albert and I brainstormed and found the best solution to be to provide additional getter methods in TopicInfo for the values in TopicDescription.

  • This will provide the same prominence to values from TopicDescription as we currently provide to values inherited from Entry.
  • A lot of mechanical work is involved in this (getters, setters, unit tests), but not much logic that could break.
  • This assumes that the values inherited from Entry are just as useful for the customer.
  • I just spoke with Abhishek Lal (Service Bus PM) about this and he is ok with exposing both Entry and TopicDescription attributes at the top level for PHP – I did point out that Java is different to him. He said these header values (from Entry) are more relevant for relay, but there’s the benefit of being consistent if we expose them across the board.

Now, another option would be to consider TopicInfo to have a Has-A relationship with Entry and Has-A with TopicDescription. We could then still add getters/setters for the TopicDescription classes. This would be the closest possible to Java in terms of experience. @Albert what do you think of this – this is an extension to the E option we had?

@antonba
Copy link
Contributor

antonba commented Jun 20, 2012

I must point out that he said it is very unlikely that the header values (the ones coming from Entry) would soon be useful in for the messaging part of SB (as opposed to relays). So let's discuss the last option I mentioned tomorrow morning.

@gcheng
Copy link
Contributor

gcheng commented Jun 20, 2012

Over the scrum, the design decision was to make both Entry and TopicDescription the 1st class citizen of the TopicInfo and expose those properties at TopicInfo level. This will have the design of TopicInfo class most closely resemble the TopicInfo class in Java.

@gcheng
Copy link
Contributor

gcheng commented Jun 21, 2012

fix ready

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants