Public bug reported:

In the Store class, there is the function that currently has the
signature:

    virtual bool getNodeReference(Item_t& result, Item* node) = 0;

Specifically, it takes an Item* (pointer to non-constant Item) because
it presumably mutates the Item object if it has to assign a node
reference.

However, the function signature should really be:

    virtual bool getNodeReference(Item_t& result, Item const* node) = 0;

that is, take an Item const* (pointer to constant Item). The operation
is "logically const." An implementation may choose not to alter the Item
at all and instead store a mapping between Items and node references
elsewhere.

However, for an implementation that does store the node reference in an
Item itself, that doesn't change the fact that the operation is still
"logically const." The node reference data member should be made
"mutable."

Having the function take an Item* as it does now means that other
classes elsewhere that return an Item const* for which you want to
obtain its node reference must first const_cast the pointer which is
ugly.

For example, the FTToken class keeps an Item const* pointing to the Item
whence the token came. There is no reason FTToken itself needs the Item
to be non-const, hence it declares its data member as Item const*.
However, in code elsewhere, we want to obtain the node reference for a
token's Item like this:

  if ( store::Item const *const token_item = token->item() ) {
    store::Item *const temp = const_cast<store::Item*>( token_item );
    if ( GENV_STORE.getNodeReference( item, temp ) ) {
      // ...
    }
  }

The introduction of the const_cast and temp is ugly and shouldn't be
necessary. It would be even more ugly to change the FTToken class to use
an Item* because then everything that passes an Item* to FTToken might
have to be changed also, i.e., the non-const-ness would ripple
throughout the code.

Note that changing the function signature to the proposed one using
const is a backwards *compatible* change because increasing "const-ness"
is always so. (The reverse, however, i.e., making something that is
const into non-const, is a backwards incompatible change.)

** Affects: zorba
     Importance: Medium
     Assignee: Markos Zaharioudakis (markos-za)
         Status: New


** Tags: item store

-- 
You received this bug notification because you are a member of Zorba
Coders, which is the registrant for Zorba.
https://bugs.launchpad.net/bugs/948259

Title:
  Store::getNodeReference() should take Item const*

Status in Zorba - The XQuery Processor:
  New

Bug description:
  In the Store class, there is the function that currently has the
  signature:

      virtual bool getNodeReference(Item_t& result, Item* node) = 0;

  Specifically, it takes an Item* (pointer to non-constant Item) because
  it presumably mutates the Item object if it has to assign a node
  reference.

  However, the function signature should really be:

      virtual bool getNodeReference(Item_t& result, Item const* node) =
  0;

  that is, take an Item const* (pointer to constant Item). The operation
  is "logically const." An implementation may choose not to alter the
  Item at all and instead store a mapping between Items and node
  references elsewhere.

  However, for an implementation that does store the node reference in
  an Item itself, that doesn't change the fact that the operation is
  still "logically const." The node reference data member should be made
  "mutable."

  Having the function take an Item* as it does now means that other
  classes elsewhere that return an Item const* for which you want to
  obtain its node reference must first const_cast the pointer which is
  ugly.

  For example, the FTToken class keeps an Item const* pointing to the
  Item whence the token came. There is no reason FTToken itself needs
  the Item to be non-const, hence it declares its data member as Item
  const*. However, in code elsewhere, we want to obtain the node
  reference for a token's Item like this:

    if ( store::Item const *const token_item = token->item() ) {
      store::Item *const temp = const_cast<store::Item*>( token_item );
      if ( GENV_STORE.getNodeReference( item, temp ) ) {
        // ...
      }
    }

  The introduction of the const_cast and temp is ugly and shouldn't be
  necessary. It would be even more ugly to change the FTToken class to
  use an Item* because then everything that passes an Item* to FTToken
  might have to be changed also, i.e., the non-const-ness would ripple
  throughout the code.

  Note that changing the function signature to the proposed one using
  const is a backwards *compatible* change because increasing "const-
  ness" is always so. (The reverse, however, i.e., making something that
  is const into non-const, is a backwards incompatible change.)

To manage notifications about this bug go to:
https://bugs.launchpad.net/zorba/+bug/948259/+subscriptions

-- 
Mailing list: https://launchpad.net/~zorba-coders
Post to     : zorba-coders@lists.launchpad.net
Unsubscribe : https://launchpad.net/~zorba-coders
More help   : https://help.launchpad.net/ListHelp

Reply via email to