-1

I'm building a Selection class that holds and allows manipulating selected Items on a canvas.

  • The Selection is part of the Document, the top-level app-container if you will.

  • I'd like to be able to clear the selection from inside the Selection class.

  • The problem is that selection creation/clearing is done on the Document.

Is it a code smell to inject the parent Document in the child Selection so I can invoke its methods from within Selection?

If yes, why and what are alternatives to this?

class Document {
  constructor() {
    this.currentSelection = null
  }

  createSelection(items) { 
    // here I'm creating a Selection and injecting
    // this `Document` within it.
    this.currentSelection = new Selection(items, this)
  }

  clearSelection() {
    this.currentSelection = null
  }
}


class Selection extends HTMLElement {
  // this class also renders the flyout buttons that follow around
  // the selection on the canvas. The buttons can call methods on
  // this class.
  constructor(items, document) {
    this.items = items
    this.document = document
  }

  updateFillColor(fillColor) {
    // updates all this.items fill color, just a placeholder for now
  }

  clear() {
    this.document.clearSelection()
  }
}

Note: The above is just an example. My actual problem is significantly more complicated (involves calling more methods on the parent), but is conceptually the same.

  • 1
    Could you clarify why you'd want to command a Document object from or via a Selection object? In your example there's a call to non existing method document.clearSelection(). That's slightly confusing. Should it be document.cancelSelection() instead? – COME FROM Jun 14 '18 at 17:50
  • @COMEFROM Yes, my bad. Let me edit the code. – Nik Kyriakides Jun 14 '18 at 17:51
  • Ask yourself: who cleans your room? you or the dirt? – Thomas Junk Jun 14 '18 at 18:24
  • @ThomasJunk My example might have been a bit bad. That doesn't mean there is no case where I would have to do this type of child -> parent communication. I'd like to know of ways to handle that. – Nik Kyriakides Jun 14 '18 at 18:49
  • @NicholasKyriakides To rephrase my hint: it is a question about responsibility. As long as your problem is abstract, the answer is: it depends. If it is concrete, you may decide for yourself. – Thomas Junk Jun 14 '18 at 19:08
1

You ask "Is it a code smell to inject the parent Document in the child Selection so I can invoke it's methods from within Selection?"

This could smell of needless coupling of the two classes, but surely there are parent-child relationships where it is sensible to pass the parent to the child object.

One question you should ask yourself is: why does the Selection need to know about or depend on Document? If Document is complex and contains methods that are of no relevance to Selection this begins to smell.

If the Selection only needed to let Document know that it has been cleared, all the selection would need is a reference to a SelectionClearedListener with the clearSelection method, which the Document class would implement and that gets called from the clear method on Selection. Thus, you could pass along the document to the selection without it knowing that it's a Document. The point is what role the document plays to the selection; in this case, someone that acts on clearing the selection.

You write that your actual problem is significantly more complicated and that the Selection needs to call more methods on the parent Document. You can follow the reasoning: what other objects does the Selection need to interact with that may actually be a Document, but which the Selection does not care to know is a Document? Those could be known to the Selection as other types of object and be passed to the Selection as separate objects. (In fact, those other objects might be implemented by different classes than Document. Dividing the responsibilities of Document into separate objects may reduce complexity.)

A relevant principle that captures the above is the Interface Segregation Principle, which can be applied to your example: "Selection should not be forced to depend on methods of Document it does not use".

In your real scenario, does the Document contain methods that Selection does not need to depend on? If so, this dependency between the two objects starts to smell.

Another consideration is that Selection may be reused for other collections of items. Again we may ask what role does the object have for the child.

  • What you are saying in the 1st paragraph is basically: 'If you want to communicate from a child to a parent, use events instead'? Or am I getting this wrong? – Nik Kyriakides Jun 16 '18 at 16:22
  • No, that is not the point I am trying to make. You could call any method on Document to perform operations etc. The point is if the parent or owner plays the role of Document to the Selection or whether its not relevant to the Selection that it is a Document. – MikkelRJ Jun 16 '18 at 17:24
  • It's not. The child simply wants to call actions on the Document. It doesn't care if it's a Document. – Nik Kyriakides Jun 16 '18 at 17:48
  • I have added to my answer. Basically I think you cannot generalize about injecting parent object to the child being a code smell. – MikkelRJ Jun 16 '18 at 18:16

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.