Changing the behavior of Extensible Items

Currently most Items within an Attribute Resource have the following properties:

  • NumberOfRequiredValues - the minimum number of values the Item must contain
  • Extensible - Can the number of values be modified?
  • MaximumNumberOfValues - the maximum number of values an extensible Item can have
  • Add/Remove Value - methods to change the number of values of an extensible Item
  • set/unset - methods of setting/unsetting a value within the Item

If an Item is not extensible, it behaves like an array. The number of values is fixed and you can only set/unset a value. If an Item is extensible, it behaves more like a vector. Note that I said “more like”.
In the current implementation, an extensible item allows values to be added as long as the number of values does not exceed the maximum number (if there is one). In the case of removing values of an extensible Item, it’s a bit more complicated. If the index of the value you wish to remove is greater than or equal to the NumberOfRequiredValues, the value will be removed. If the index is less than the NumberOfRequiredValues it will only be unset. The original reason was to treat indices < NumberOfRequiredValues like an array.

This design has caused complications since consuming code needs to address both the array and vector behaviors differently. In order to simplify things I propose the following changes:

RemoveValue Can Remove Any Index

The first change is the simplest and should cause no (or minimal) issues. Let the RemoveValue method be able to remove any value (regardless of its position w/r to the NumberOfRequiredValues as long as the resulting number of values >= the NumberOfRequiredValues. This would remove the special treatment of the “array” section of the Item in terms of value removal. Note that if the removal would result in the NumberOfRequiredValues being violated, it would do an unset instead (which is similar to what is done currently.

Let Extensible Items Behave As Vectors

This one extends the above change in the following way for Items marked Extensible:

  • Extensible Items start with NumberOfValues = 0 instead of NumberOfRequiredItem
  • RemoveValue will always be able to remove a value regardless of it location or the number of values in the Item
  • RemoveValue will not unset values if used on a non-extensible Item
  • The Item’s isValid method will return false if the number of values < NumberOfRequiredValues

With these changes an Item will either act as an Array (non-extensible) or as a Vector (extensible) instead of the strange hybrid we currently have (Extensible Items with NumberOfRequiredValues != 0)

Note: This change could result in some behavior issues if the consuming code always assumes that the Item must have at least NumberOfRequiredValues instead of checking the Item’s NumberOfValues.

2 Likes

@chart3388 @amuhsin @dcthomp @johnt @tj.corona - feel free to comment!

I prefer extensible items behaving as vectors.

Also, it’s perhaps tangential but might alleviate some of the “design pressure” on the API if there was a method to “set any unset item or extend the item as required.” We currently have setValue() that requires you to choose an index and appendValue() which (AFAIK) always attempts to increase the item’s storage.

If items had an additional std::size_t m_nextUnused ivar, then a method that filled unused entries before expanding would be efficient and help prevent a lot of faux pas that get committed with vector-style storage that also allows unset elements. It would help callers avoid checking item size when writing to the item.

Perhaps a similar “read” method for extensible items would help reduce errors by performing the check internally rather than requiring 2 calls to fetch a value.

template<typename T>
class ValueItemTemplate : ...
{
public:
  /// If \a index exists and value at \a index is set,
  /// returns true and sets data. Otherwise, returns false.
  bool value(std::size_t index, T& data);

  /// Sets the first available unset value to \a vv or extends
  /// the item (if possible) and inserts \a vv at the end.
  /// Returns true if the item was modified, false otherwise.
  bool addValue(T& vv);

protected:
  /// Index of first unset value in the item (or numberOfValues()).
  ///
  /// Must be maintained by setValue, unset, reset,
  /// appendValue, addValue, etc. but amortized cost is very low.
  std::size_t m_firstUnset;
};