2

Even though I'm programming in PHP, I'm open to reviewing language-agnostic suggestions, as they might point me to valuable directions.

To remove any possible confusion I feel some comments appear to allude to: the Pages in this question do not refer to web pages, but to in-memory pages for an API akin to word processing and/or a spreadsheet.

I have a Page class whose instances can be part of a Pages collection. These Page objects must be uniquely named if they're part of a Pages collection, as they must be uniquely identifiable by their name.

A Page object can never belong to multiple Pages collections at the same time: they will either be moved, or be copied (cloned).

Consumers will also be able to alter the name of individual Page objects, for instance with1:

$page = new Page( 'Optional name' );
// or ($pages is a Pages collection instance)
$page = $pages->getByName( 'Page 1' );

// and then
$page->setName( 'My Page' );

If a Page is added to a Pages collection, the collection will ensure the Page is renamed, if its name conflicts with another Page in the collection, like so:

class Pages
{
  private $objectIndex;
  private $nameIndex;

  public function __construct() {
    $this->objectIndex = new SplObjectStorage;
    $this->nameIndex = [];
  }

  public function add( Page $page ) {
    if( !$this->objectIndex->contains( $page ) ) {
      $name = $page->getName();
      if( isset( $this->nameIndex[ $name ] ) ) {
        // make name unique to this collection
        $name = $this->someLogicToMakeNameUnique( $name );
        // alter name
        $page->setName( $name );
      }
      $this->nameIndex[ $name ] = $page;
      $this->objectIndex->attach( $page, $name );
    }
  }

  public function getByName( $name ) {
    if( isset( $this->nameIndex[ $name ] ) ) {
      $this->nameIndex[ $name ];
    }

    return null;
  }
}

I'm trying to come up with a strategy to make sure that, if a Page is part of a collection and its name is altered, the name automatically gets adjusted to a unique name (i.e. if "Page 1" already exists, rename it to "Page 2") and that the Pages::$nameIndex keys get properly updated as well (as this allows for faster retrieval of a Page by name than looping through it's contained Pages until a name matches).

Strategies I've come up with so far are:

  1. Pass Pages collection to Page::setParent( Pages $container ) and call some verification/sanitation mechanism on $this->container in Page::setName( $name ), before altering.

  2. Emit a rename event from Page::setName( $name ) and have Pages do something like $event->preventDefault() if the name conflicts and then have Pages alter the name to something unique instead.

Option 1 seems the easiest/laziest/least process intensive, but puts the responsibility of verifying uniqueness in an object where it does not belong.

Option 2 appeals the most to me so far, but it has a downside as well: It will emit multiple rename events, if the Pages collection needs to alter the name itself again.

NB: Even though both the Page::setParent( Pages $container ) (as a Page can only ever be owned by one collection) and event dispatching capabilities (primarily meant for consumer purposes) are already implemented, neither are utilized for my unique naming conundrum yet.


Do you have any other suggestions that satisfy the constraints that Pages should be responsible for verifying uniqueness and that there's not to much back-and-forth communication necessary?


1) As a reference: my objective is akin to Excel VBA's Workbook.Sheets("Sheet 1").Name = "Sheet" type of behavior, which I believe behaves similar to what I am after.

  • 1
    Can a Page object belong to more than one Pages collection? Or is there always a unique "parent container" of a page? – Doc Brown Jan 7 '18 at 9:18
  • @DocBrown Good question! I forgot to mention that a Page can never belong to multiple Pages collections at once. I'll update my question with this information. – Decent Dabbler Jan 7 '18 at 9:28
  • 1
    I am too lazy to write an actual answer, but created a PHP snippet for you to look at. The example deals with the circular reference while keeping the responsibility of uniqueness in the place where it belongs, in the Pages object. – Andy Jan 7 '18 at 10:37
  • 4
    Does the name of a page even make sense outside of a pages collection? Shouldn't the name be a property of the association between a pages collection and a page instead of a property of the page? Similar to how the index of an object in an array is a property of the "being in an array" and not a property of the object? – Jörg W Mittag Jan 7 '18 at 11:14
  • @DavidPacker Hahaha, holy sh...! Too lazy to write an answer, but gives a full-fletched solution instead. Hahaha, I like your style, man. And I really feel bad for saying this, as I see what you are doing and I'm sure it works as expected, but unfortunately it uses completely different paradigms than what I am envisioning: your solution revolves around the repository pattern (and the strategy pattern, which may actually be useful), but I feel this would be too cumbersome for my intended API-consumers, unless I can somehow encapsulate this behind my API. ... – Decent Dabbler Jan 7 '18 at 11:15
5

Uniqueness of those names is a property which is clearly defined only in context of a Pages collection. That is most easiest understood when you think of a Page object changing its "parent" container, or a Page object with no parent container at all.

Thus verifying uniqueness is clearly a responsibility of the Pages class, not of a Page. As a consequence, when the verification/sanitation mechanism is implemented in Pages, it is exactly where it belongs to. Calling this mechanism from Page::setName does not change the responsibility, so your "Option 1" is perfectly ok.

As a side note: the most simple "verification/sanitation mechanism" I can think of is to simply forbid any name change of a Page once it has been assigned to a Pages collection. This simple strategy might be totally sufficient for several use cases. If a page has to be renamed, it needs to be taken out of the collection first, then renamed, and then added again by using the add method in your example.

  • Thanks for you answer. I totally agree with your first paragraph and the first sentence of your second paragraph, about it being Pages responsibility. But that's exactly why I'm not too fond of my first option: I feel that a Page should not at all be concerned with name uniqueness and it should not be a Page's responsibility to verify with its parent whether a name is allowed. And addressing your last paragraph: I'd rather retain that functionality for consumer-friendliness, especially since they would otherwise also have to remember its position (as the collection is ordered as well). – Decent Dabbler Jan 7 '18 at 10:02
  • @DecentDabbler: ok, it seems I misunderstood your question. It seems when you wrote "puts the responsibility of verifying uniqueness in an object where it does not belong" you did not mean the Pages object, but the Page object. But to my understanding, the responsibility is in the Pages object in this solution, the fact the validation is called from within the Page object does not change this. – Doc Brown Jan 7 '18 at 11:09
  • @DecentDabbler: see my edit. – Doc Brown Jan 7 '18 at 11:14
  • Yeah, sorry about that confusing bit; I was talking about Page indeed. And thanks again: I may actually end up using this after all if no other viable solutions come up or if other solutions end up being too cumbersome. At a minimum I'm going to ponder on this option some more. – Decent Dabbler Jan 7 '18 at 11:39
  • 1
    @DecentDabbler: either this, or you use the "null object pattern" where you have a special null object of type Pages which is used as a default parent for each page as long as there is no "real" parent container. For this object, the renamePage method does nothing than simply. assigning the new name. Not sure if this is overengineered for your case. If the "renamePage" turns out to be the only use case for the "null object", I would probably not go this route. If, however, you get a lot of if( $this->hasParent(), then it makes sense. – Doc Brown Jan 7 '18 at 12:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.