Issue 16050: duplicate put definition resulting in a name clash
Issue 16056: Data access from DataReader using java.util.List
Issue 16104: Missing -behavioural- descriptions of the interface
Issue 16316: Improve usability of “bucket” accessors
Issue 16317: Update specification to reflect DDS-XTypes FTF1 issue resolutions
Issue 16318: Entity.setListener is missing listener mask
Issue 16319: Unclear blocking behavior for WaitSet.waitForConditions overloads that don’t specify timeout
Issue 16320: Incorrect topic type specification in DomainParticipant.createMultiTopic
Issue 16321: Too many read/take overloads
Issue 16322: DynamicDataFactory.createData missing a parameter
Issue 16323: Logically ordered types should implement java.lang.Comparable
Issue 16324: Improve polymorphic sample creation
Issue 16325: Remove unnecessary DataWriter.write overloads
Issue 16326: copyFromTopicQos signatures are not correct
Issue 16327: Parent accessors should be uniform across Entities and Conditions
Issue 16328: DataReader.createReadCondition() is useless
Issue 16369: QosPolicy.Id enumeration is redundant
Issue 16530: Superfluous "QoSPolicy" Suffix on Policy Types
Issue 16532: RxO QoS Policies should be Comparable (idem for QoS)
Issue 16533: Qos Policies ID class vs. numeric ID
Issue 16534: Constant Values SHALL be defined by the standard
Issue 16537: Get rid of the EntityQos Class
Issue 16538: Entity class allows for breaking invariants
Issue 16539: DomainEntity should be removed
Issue 16540: DataReader API
Issue 16541: A Status is not an Event. An Event is not a Status, it notifies a status change
Issue 16542: Avoid unncessary side-effects on the DataWriter API
Issue 16543: Statuses API shoud be improved and made typesafe
Issue 16588: The Sample class should provide a method to check wether the data is valid
Issue 16589: Misnamed Listener Helper
Issue 16050: duplicate put definition resulting in a name clash (dds-psm-java-ftf)
Click here for this issue's archive.
Nature: Clarification
Severity: Significant
Summary: The ModifiableEntityQos contains put() definition that, after type erasure, cannot be distinguished from the inherited put definition in EntityQos (or the one inherited from Map) resulting in duplicate definitions of put: QosPolicy<?,?> put(QosPolicy.Id, QosPolicy<?,?>)
Possible ways to resolve this:
·Drop the “extends Map” in EntityQos and put a dedicated get() in EntityQos and a dedicated put()/set() in ModifiableEntityQos and leave it up to the implementation on how to manage these values. This is the preferred solution as it prevents the user of the API to accidently use the Map inherited modification methods like put/remove/clear on a non-modifiable EntityQos.
·Modify the signature of put() in ModifiableEntityQos to match the inherited definitions in EntityQos and Map:
public QosPolicy<?,?> put(QosPolicy.Id key, QosPolicy<?,?> value);
Resolution: See the modifications described below.
Revised Text: See revision #161, which rolls up the revisions above and also resolves issue #16369: http://code.google.com/p/datadistrib4j/source/detail?r=161. These changes are also available in the attached file diff_omg_issue_16050_16369.txt.
Actions taken:
March 10, 2011: received issue
April 10, 2012: closed issue
Discussion: André] Possible ways to resolve this:
1. Drop the “extends Map” in EntityQos and put a dedicated get() in EntityQos and a dedicated put()/set() in ModifiableEntityQos and leave it up to the implementation on how to manage these values. This is the preferred solution as it prevents the user of the API to accidently use the Map inherited modification methods like put/remove/clear on a non-modifiable EntityQos.
2. Modify the signature of put() in ModifiableEntityQos to match the inherited definitions in EntityQos and Map:
public QosPolicy<?,?> put(QosPolicy.Id key, QosPolicy<?,?> value);
[Rick] I think the Map extension provides a useful way to navigate QoS objects in a generic way. Therefore, I prefer the second approach.
See also issue #16369, which will potentially impact the same method signature. See also issue #16537, which proposes eliminating the EntityQos interface altogether.
See the following revisions, which contain provisional fixes for this issue:
• Revision #116: http://code.google.com/p/datadistrib4j/source/detail?r=116, which updates the method signature as described in (2) above and also addresses issue #16369.
• Revision #136: http://code.google.com/p/datadistrib4j/source/detail?r=136, which further refines the method signature to address a compilation error introduced by Java 7 compliance.
Issue 16056: Data access from DataReader using java.util.List (dds-psm-java-ftf)
Click here for this issue's archive.
Nature: Enhancement
Severity: Minor
Summary: Currently the DataReader provides read() and take() methods that return a special type of java.util.ListIterator: Sample.Iterator. The Iterator is not the most convenient way to access data retrieved from the DataReader (e.g. an Iterator can only be traversed once).
Propose to modify all read()/take() operations currently returning an Iterator to let them return a java.util.List. The List is more developer friendly, as it can be traversed multiple times and a List is also an Iterable with the added benefit that it can be used in Java’s “foreach” statement:
List<Sample<TYPE>> data = dataReader.read();
for (Sample<Type> sample : data) {
// ...
}
Resolution: Merge this issue with #16321, which proposes other changes to the read/take overloads.
• Overloads that return a loan should do so in the form of a ListIterator implementation, which will allows multiple forward and backward navigation of elements. The loaned samples should not be returned as a List, as retrieving an iterator from a list would force critical-path memory allocation—in direct contradiction of the low-latency goal of the loaning operations.
• Overloads that return a copy should continue to accept a List to be filled in and should return a reference to the same list for convenience. These overloads will therefore support the “for-each” construct requested by this issue.
Disposition: Merged with issue #16321
Revised Text:
Actions taken:
March 10, 2011: received issue
April 10, 2012: closed issue
Discussion:
Issue 16104: Missing -behavioural- descriptions of the interface (dds-psm-java-ftf)
Click here for this issue's archive.
Nature: Clarification
Severity: Significant
Summary: Some parts of the interface (javadoc) are poorly documented especially with respect to behaviour. This Java documentation will be the key documentation for the -new- DDS application programmers. It may be trivial or implicit for the ones writing the standard but it will not be for the application programmers which are not familiar with the existing DDS standard will use it
For example have a look at the method createDataWriter(Topic<TYPE> topic) on the Publisher interface. What will happen if the middleware cannot create the datawriter. Will an unchecked exception be thrown or is a null value returned or even worse the datawriter is simply returned and will fail when the first write action is performed?
I now that the existing OpenSplice DDS implementation will return null when the middleware is not able to create the datawriter but it would be nice that applications are not only portable from interface compliance aspect but also from behavioural aspect! (and that application programmers are aware of it)
Resolution: Merge the descriptions of classes and operations from the DDS specification into the appropriate JavaDoc comments. This PSM does not introduce new concepts, so no merge is necessary in that case. DDS-XTypes is in finalization, so its contents are not yet fixed. Therefore, to avoid the possibility of errors and inconsistencies, we should put it aside for now.
Revised Text: The contents of the DDS specification have been merged into JavaDoc comments in revision #140: http://code.google.com/p/datadistrib4j/source/detail?r=140. The difference is also available in the attached file diff_omg_issue_16104.txt.
Actions taken:
March 31, 2011: received issue
April 10, 2012: closed issue
Discussion: [Rick] The behavioral specifications already exist—in the appropriate specification documents: DDS, DDS-XTypes, and DDS-PSM-Java. So I think this issue is not about portability, but really about ease of use: it is more convenient to programmers if more of the relevant specifications are available copied into the JavaDoc.
Issue 16316: Improve usability of “bucket” accessors (dds-psm-java-ftf)
Click here for this issue's archive.
Source: DECA (Mr. Rick Warren, )
Nature: Uncategorized Issue
Severity:
Summary:
The third bullet at the end of section 7.1.5, “Method Signature Conventions”, reads: · Accessors for properties that are of mutable types, and that may change asynchronously after they are retrieved, are named get<PropertyName>. They take a pre-allocated object of the property type as their first argument, the contents of which shall be overwritten by the method. To facilitate method chaining, these methods also return a reference to this argument. This pattern forces the caller to make a copy, thereby avoiding unexpected changes to the property. An Entity’s status is an example of a property of this kind. (This pattern of passing a container to an object for that object to “fill in” has sometimes been referred to as a “bucket” pattern.) In cases where object-allocation performance is not a significant concern, the usability of this pattern can be improved with a trivial addition: allow the caller to pass in a null “bucket”, and require the implementation to allocate and return a new object with the appropriate contents. Proposed Resolution: Add a sentence to the bullet that indicates that a null argument is permitted. Proposed Revised Text: Replace the third bullet in section 7.1.5, “Method Signature Conventions” with the following: · Accessors for properties that are of mutable types, and that may change asynchronously after they are retrieved, are named get<PropertyName>. They take a pre-allocated object of the property type as their first argument, the contents of which shall be overwritten by the method. To facilitate method chaining, these methods also return a reference to this argument. The caller may alternatively pass a null argument into such accessor methods, in which case the implementation shall allocate a new object, set its contents appropriately, and return it. This pattern forces the caller to make a copy, thereby avoiding unexpected changes to the property. An Entity’s status is an example of a property of this kind.
Discussion: [Rick] To resolve this issue by itself, we could add a sentence to the bullet that indicates that a null argument is permitted. Specifically, replace the third bullet in section 7.1.5, “Method Signature Conventions” with the following: • Accessors for properties that are of mutable types, and that may change asynchronously after they are retrieved, are named get<PropertyName>. They take a pre-allocated object of the property type as their first argument, the contents of which shall be overwritten by the method. To facilitate method chaining, these methods also return a reference to this argument. The caller may alternatively pass a null argument into such accessor methods, in which case the implementation shall allocate a new object, set its contents appropriately, and return it. This pattern forces the caller to make a copy, thereby avoiding unexpected changes to the property. An Entity’s status is an example of a property of this kind. Resolution: Issue #16587 also deals with bucket accessors and has a broader scope. This issue should be merged with that one. Disposition: Merged with issue #16587
Source: RTI (Rick Warren, rick.warren@rti.com) Summary: The (first) DDS-XTypes FTF has completed. Some of the issue resolutions result in API changes that impact this specification. Those changes should be reflected here to keep this specification aligned with the PIM. These issues potentially include: · #15689, Identifiers TypeId and Module collide with IDL keywords · #15691, Unclear member names when programming language doesn’t support typedef · #15693, Extensibility kinds of new QoS policies are not specified in a consistent way · #15696, Incorrect FooDataWriter overloads for built-in types · #15706, Reduce size of DynamicData API Note that this issue applies to DDS-PSM-Cxx too.
Source:
RTI (Rick Warren, rick.warren@rti.com)
Summary:
The method signature for Entity.setListener does not include the listener “mask” (actually, a collection of status classes in this PSM) parameter from the PIM.
Discussion:
The current signature is useful as a convenience for the common case where the application wants all callbacks. But it lacks the expressiveness of the PIM, so an additional overload should be provided.
Proposed Resolution:
Add the following method to the Entity interface:
public void setListener(
LISTENER listener,
Collection<Class<? extends Status<?, ?>>> statuses);
Include the appropriate JavaDoc copied from the DDS specification
The current signature is useful as a convenience for the common case where the application wants all callbacks. But it lacks the expressiveness of the PIM, so an additional overload should be provided.
Source: RTI (Rick Warren, rick.warren@rti.com) Summary: The method WaitSet.waitForConditions is provided with several overloads, including some that do not take an explicit timeout. These are intended to wait indefinitely. However, they still throw TimeoutException. How can they time out if they wait forever? Discussion: Object.wait allows indefinite waiting, so it makes sense for this specification to allow it as well. However, these overloads should not ever throw TimeoutException. Proposed Resolution: Remove the clause “throws TimeoutException” from these method declarations.
Object.wait allows indefinite waiting, so it makes sense for this specification to allow it as well. However, these overloads should not ever throw TimeoutException.
The method DomainParticipant.createMultiTopic specifies the type of the resulting object using a registered type name in string form. However, this is inconsistent with the way type registration is handled elsewhere in this PSM: callers provide a Class or TypeSupport object, and the implementation registers the type implicitly as necessary. Discussion: We should follow the model of createTopic: provide two overloads, one taking a Class and the other a TypeSupport. Proposed Resolution: Replace the existing createMultiTopic method declaration with two new overloads. In place of the typeName string, the first new overload shall take a Class parameter. The second shall take a TypeSupport parameter
We should follow the model of createTopic: provide two overloads, one taking a Class and the other a TypeSupport
The DataReader interface defines a very large number of read and take variants. While each one has a clear meaning, the sheer number of them makes the API harder to understand. Discussion: One possibility would be to follow the example of the C++ PSM, and combine things like condition, handle, etc. into a “filter” parameter. Note that this issue should be resolved at the same time as #16056.
[Rick] One possibility would be to follow the example of the C++ PSM, and combine things like condition, handle, etc. into a “filter” parameter. [Angelo] The only overload for the createReadCondition should be accepting a parameter that is the same as the DataReader.read e.g. a Query. This way the API will be more consistent. See the following revisions, which constitute a provisional resolution to this issue: • Revision #131: http://code.google.com/p/datadistrib4j/source/detail?r=131, which collapses view states, instance states, sample states, and other parameters into helper types Subscriber.ReaderState and DataReader.Query. • Revision #135: http://code.google.com/p/datadistrib4j/source/detail?r=135, which eliminates mandatory memory allocations in some overloads. • Revision #152: http://code.google.com/p/datadistrib4j/source/detail?r=152, which makes the signatures of createReadCondition and createQueryCondition consistent with the new signatures of read and take.
Source: RTI (Rick Warren, rick.warren@rti.com) Summary: According to DDS-XTypes, the operation DynamicDataFactory.create_data (createData in this PSM) takes a parameter that indicates the DynamicType of the new data object to create. That parameter is missing, leaving implementations with no way to determine—and applications with no way to specify—the type of the created object. Proposed Resolution: Add the missing argument. Proposed Revised Text: Apply the following difference to the method signature: - public abstract DynamicData createData(); + public abstract DynamicData createData(DynamicType type);
Source: RTI (Rick Warren, rick.warren@rti.com) Summary: Several of the types defined in this PSM have a natural order (such as Time). In order to better integrate with the Java platform, these types should implement the standard java.lang.Comparable interface. Proposed Resolution: Implement Comparable in the following types: · Bit—ordered based on index within a bit set · Duration—ordered from shorter durations to longer ones · Time—ordered from earlier points in time to later ones
See the following revisions, which constitute a provisional resolution to this issue: • Revision #122: http://code.google.com/p/datadistrib4j/source/detail?r=122. This update encompasses Bit, Duration, and Time. • Revision #134: http://code.google.com/p/datadistrib4j/source/detail?r=134. This update encompasses InstanceHandle.
Source: RTI (Rick Warren, rick.warren@rti.com) Summary: The specification does not provide a simple, portable way to create a new data sample to use with the middleware. Instead, there are several partial solutions: · Instantiate a concrete sample type directly: “new Foo()”. This approach doesn’t work in generic methods—i.e. when the concrete type is not statically known. It also doesn’t work with DynamicData. · Instantiate DynamicData from DynamicDataFactory. But samples of statically known, user-defined types don’t have a “data factory”. · Use DataReader.createData(). But there is not equivalent on the publishing side. There should be a single way that works uniformly and generically. Proposed Resolution: The proposed resolution has several parts: 1. Introduce a new factory instance method to the TypeSupport class: TypeSupport.newData(). The name of this method is parallel to that of other value type “constructor-like” factories. 2. Support navigation from the TopicDescription to the TypeSupport. Add a new method TopicDescription.getTypeSupport(). 3. Simplify the number of ways to get from the data type’s TypeSupport to its Class. Add a method TypeSupport.getType(). Remove the existing methods TopicDescription.getType(), DataWriter.getType(), and DataReader.getType(): they are redundant. 4. Remove the existing method DataReader.createData() and the existing class DynamicDataFactory. They are not needed. There will therefore be a single polymorphic and generic way to instantiate a sample of any type: by using its TypeSupport. (You can get the TypeSupport from any related TopicDescription, or transitively any DataReader or DataWriter.) Likewise, there will be a single polymorphic and generic way to get the Class object for any data type: from its TypeSupport. As described in the previous paragraph, you can get to the TypeSupport from a variety of places.
Title: Remove unnecessary DataWriter.write overloads Source: RTI (Rick Warren, rick.warren@rti.com) Summary: The specification currently provides overloads for DataWriter.write that take the following combinations of parameters 1. The sample to write, without an instance handle. (If the type is not keyed, no instance handle is necessary. If it is keyed, the instance handle is implicitly nil and will be inferred by the implementation.) 2. The sample to write, without an instance handle but with a time stamp. 3. The sample to write, with an instance handle. 4. The sample to write, with both an instance handle and a time stamp. The overloads would be easier to understand if they formed a progression from fewer parameters to more. We can do this by removing (2). Proposed Resolution: Remove the following methods: - public void write( - TYPE instanceData, - Time sourceTimestamp) throws TimeoutException; - public void write( - TYPE instanceData, - long sourceTimestamp, - TimeUnit unit) throws TimeoutException; Also, update the documentation of the remaining overloads to clarify that if the topic is not keyed, they can be called with a nil or null InstanceHandle.
[Rick] Proposal: Remove the following methods: - public void write( - TYPE instanceData, - Time sourceTimestamp) throws TimeoutException; - public void write( - TYPE instanceData, - long sourceTimestamp, - TimeUnit unit) throws TimeoutException; Also, update the documentation of the remaining overloads to clarify that if the topic is not keyed, they can be called with a nil InstanceHandle. [Virginie] The instance handle on one hand and the timestamp on the other hand are two orthogonal parameters; I find it rather confusing to introduce here a “fake ordering” between those two things that are totally unrelated. If we were to remove unnecessary overloads, why not selecting just one way to express timestamps instead of the two currently existing? [Angelo] Not sure this is the right approach of reducing overloads. Perhaps the approach is to have only one way of passing a time object. Yet, it is better to avoid API that allow for parameters to be NIL or null. It is not clear that the current set of overloads is a problem. This issue is closed without any changes. Disposition: Closed, no change
Proposed Resolution: Change the signatures as follows: In Publisher.java: - public void copyFromTopicQos(DataWriterQos dst, TopicQos src); + public ModifiableDataWriterQos copyFromTopicQos( + ModifiableDataWriterQos dst, TopicQos src); In Subscriber.java: - public void copyFromTopicQos(DataReaderQos dst, TopicQos src); + public ModifiableDataReaderQos copyFromTopicQos( + ModifiableDataReaderQos dst, TopicQos src);
See revision #126: http://code.google.com/p/datadistrib4j/source/detail?r=126, which constitutes a provisional resolution to this issue. Resolution: Change the signatures as follows: In Publisher.java: - public void copyFromTopicQos(DataWriterQos dst, TopicQos src); + public ModifiableDataWriterQos copyFromTopicQos( + ModifiableDataWriterQos dst, TopicQos src); In Subscriber.java: - public void copyFromTopicQos(DataReaderQos dst, TopicQos src); + public ModifiableDataReaderQos copyFromTopicQos( + ModifiableDataReaderQos dst, TopicQos src);
Source: RTI (Rick Warren, rick.warren@rti.com) Summary: All DomainEntity interfaces, and some Condition interfaces, can provide a reference to the parent object. In the case of Entities, this accessor has been captured in the form of a generic base interface method: PARENT DomainEntity.getParent(); However, StatusCondition and ReadCondition are not parallel. They provide the following methods: ENTITY StatusCondition.getEntity(); DataReader<TYPE> ReadCondition.getDataReader(); It would be more consistent if we renamed both of the above methods to getParent. Proposed Resolution: Rename StatusCondition.getEntity to getParent. Rename ReadCondition.getDataReader to getParent.
Source: RTI (Rick Warren, rick.warren@rti.com) Summary: The DataReader interface provides two overloads for the createDataReader method: one that takes no arguments and another that takes the appropriate sample states, view states, and instance states. The existence of the first overload supposes that it will be common to create a ReadCondition with any sample state, any view state, and any instance state. But in fact, such a ReadCondition is not very useful at all: there’s no point in passing it to read/take, because it will not filter the available samples in any way. And although you could use it with a WaitSet, it doesn’t do anything that you couldn’t do with a StatusCondition on the DATA_AVAILABLE status. Proposed Resolution: Remove the no-argument overload of DataReader.createReadCondition. Leave the three-argument overload unchanged.
In the DDS PIM, each QoS policy has a name and an ID that uniquely identify it. In this PSM, these two things are encapsulated in the enumeration QosPolicy.Id. But the Java platform already provides equivalent information: the Class object. The ability to quickly compare Class object pointers is equivalent to comparing ID integer values, and each Class already has a name string. Proposed Resolution: Remove the enumeration QosPolicy.Id. Replace its uses with Class<? extends QosPolicy>.
See the following revisions, which contain a provisional resolution to this issue: • Revision #116: http://code.google.com/p/datadistrib4j/source/detail?r=116 • Revision #121: http://code.google.com/p/datadistrib4j/source/detail?r=121
The dds-psm-java uses a superfluous Policy suffix to name the DDS policies which themselves are already included in a "policy" namespace. [Resolution] This suffix should be removed.
Some of the DDS QoS Policies are Request vs. Offered in the sense that the value of matching policies on communicating entities have to satisfy a specific ordering relationship. Specifically, the policy set on the receiving side should always be less or equal than the analogous QoS Policy on the sending side. As a result there is a natural total ordering for RxO Policies which is not currently captured nor reflected in the API. As a consequence also QoS should be defining a total order. [Resolution] Ensure that all RxO Policies implement the Comparable interface. This is pretty logical as for this QoS Policies it has to be established a total order. Let QoS classes also implement a comparable interface.
[Angelo] Proposal: Ensure that all RxO Policies, and all entity QoS types, implement the Comparable interface.
[Rick] There is no total order over QoS types, because they consist of multiple QoS policies, which may not all share the same relative ordering.
Re: only QoS policies then, it is true that RxO contracts define an ordering of policies. But saying that a writer with a certain policy offers a “greater” level of service than a reader with another policy is a very different sort of comparison than saying that one integer is “greater” than another. Therefore, I am not completely comfortable with this issue, although I am interested to hear what others have to say.
Therefore, I propose that, rather than implementing the Comparable interface, these policy types should provide a Comparable object. This approach is functionally identical but clarifies which comparison contract is being used. For example:
if (dwPolicy.requestedOfferedContract().compare(drPolicy) >= 0) {
System.out.println("Compatible!");
}
See revision #155: http://code.google.com/p/datadistrib4j/source/detail?r=155, which contains an initial provisional expression of this resolution. See also revision #167: http://code.google.com/p/datadistrib4j/source/detail?r=167, which adds support for DDS-XTypes-derived policies.
QoS Policies define a nested ID class for capturing the Policy ID and PolicyName. [Resolution] Remove the ID class and (1) use Java introspection for accessing the policy name, and (2) define an integral ID for specifiying the policy ID. Notice that getid and getName methods are also needed.
Constant values such as the infinite duration, etc. should be defined by the standard as opposed than the implementation. [Resolution] Define constants as part of the API.
Infinite duration and other well-defined values are defined by the standard. The class defines an operation infiniteDuration: Duration. Furthermore, the documentation of the getDuration reads, “If this duration is infinite, this method shall return Long.MAX_VALUE….” Similar operations and documentation are given for invalid Time, nil InstanceHandle, and so on.
The Entity provides some generic methods that seem of doubtful usefulness but then on the other end open up a door for messing up with the invariant of a type or at least raising runtime errors. For instance via the Entity type I can add a non-applicable QoS policy to a DDS entity -- this seems weakening the API. [Resolution] Remove all method that might break invariants such as setQos, setListener, etc.
[Angelo] Proposal: Remove all method that might break invariants such as setQos, setListener, etc. [Rick] This issue is unclear to me. It does not indicate which methods it considers unimportant, nor does it indicate how type safety may be weakened.
What is the value of having the DomainEntity class? [Resolution] The DomainEntity class should be removed and the getParent method should be migrated to the Entity class.
[Angelo] The DomainEntity class should be removed and the getParent method should be migrated to the Entity class. The DomainEntity is not on any other PSM, thus I don't see why it should be on the Java PSM. Even at a PIM level it is a class w/o attributes and w/o operations—thus a very debatable abstraction.
The read API currently seems a bit too complicated. In some in some instances
it provides part of the results as a return value and the rest by means of arguments.
This does not feel right and again violates one of the key goal of having a new PSM: simplcity
and usability.
The API does not provide a way of deciding if one wants to read/take only valid
data. This is a remark true in general for DDS which needs to be fixed for all PSM
as well as for the PIM!
The following methods on the DataReader interface are superfluous:
- cast
- createSample
[Resolution]
Refactor and cleanup the data-reader API.
The org.omg.dds.core.status.Status class currently extends the java.util.EventObject.
The issue I have with this is that a status and an event are to different concepts.
A status represents a continuous value or set of values which are always defined,
while an event represents and happening. For instance an event could be used to notify
the change of status but not the status itself.
[Resolution]
That said the refactoring suggested is to re-organize the current status types
so to clearly distinguish what is are statuses and what are the events. As such,
all the status currently defined should remove reference to the source. Why?
Because the statuses are retrieved from the source thus it is kind of silly to
add back the source on the communication status.
Let me give you an example ("dr" below is a DataReader):
RequestedDeadlineMissedStatus s = dr.getRequestedDeadlineMissedStatus();
// this give back the reader we already know, thus it is not real useful
// information which should simply be removed.
s.source())
BTW the status types as well as the relative accessor methods should
drop the trailing "Status" as it is not so informative.
That said, we should add an event type associated to each status defined like this:
class RequestedDeadlineMissedEvent {
private RequestedDeadlineMissed status;
private DataReader source;
//... useful methods
}
The event type is the one that should be used as a parameter of the listener methods.
Finally, it is worth noticing that the suggested refactoring will fix the
DataAvailableStatus anomaly. This type currently defined as a status is actually
an event and as such should be treated. So where is the anomaly, for this status
there are no methods on the data reader and there is really no status information
such as saying... Yes there are 15 new samples or something like this.
The methods that access the communication statuses should simply return an object as opposed to also require it as an argument. As the Status is immutable there is no risk in the client code changing it, thus no copies required! [Resolution] Apply the changes suggested in the description above.
The dds-psm-java currently passes statuses via collections. However this does not make it neither efficient nor simple to represent collections of related statuses, such as the Read Status, etc. [Resolution] The suggestion is to add a ReadState as currently present on the dds-psm-cxx to simplify the API and make it simpler to support the most common use cases.
The Sample Class as defined in the Beta1 of the DDS-PSM-Java does not define
a method "isValid(): Boolean" to check wether the data is actually valid. This is a simple
to fix oversight.
Resolution
---------------
Add an "isValid () : Boolean" method to the Sample Class
[Angelo] An implementation could decide to provide the last known value. Would be useful to separate the null from the strictly valid data.
The name of the classes DataReaderAdapter/DataWriterAdapter are misleading since what they are really providing are listeners with some default behaviour. Resolution --------------- Rename the classes DataReaderAdapter/DataWriterAdapter to SimpleDataReaderListener and SimpleDataWriterListener. For the SimpleDataReaderListener one could implement trivially all the method but the one that notifies the availability of data, e.g. <onDataAvailable>
[Angelo] Proposal: Rename the classes DataReaderAdapter/DataWriterAdapter to SimpleDataReaderListener and SimpleDataWriterListener. For the SimpleDataReaderListener one could implement trivially all the method but the one that notifies the availability of data, e.g. onDataAvailable.