4

Sometimes I use some indirect properties to set or get some other sub-properties,

I give an example

class Page { public string MainText { set { MainParagraph.Text = value; } get { return MainParagraph.Text; }} public TextBlock MainParagraph {set; get;} public TextBlock FootNotes {set; get;} } class TextBlock { public String Text {set; get;} public bool RightToLeft {set; get;} }

Without the MainText property of the Page I have to use myPage.MainParagraph.Text = something and now I use myPage.MainText = something, however one may still use the first alternative. (Should I prevent it?)

Please note Text is not a real property of Page and in this way it may represent some sub-properties but not all of them. Is there any trade off?

For example for the RightToLeft property I prefer myPage.FootNote.RightToLeft = true because it's an specific attribute of each TextBlock but MainText could still be known as the main text of each page.

Is it better to make FootNotes public, but MainParagraph private? Then do I need to define a property in the Page for every property of the MainParagraph?

Improve this question
4
  • 2
    are you aware of Law Of Demeter? Commented Mar 1, 2015 at 11:56
  • @gnat thank you, then not only it is more convenient but is recommended too! Commented Mar 1, 2015 at 12:04
  • No... Hiding a Law of Demeter violation behind a property call does not really fix the problem the Law aims to prevent. Commented Mar 1, 2015 at 18:48
  • 1
    @gnat My question is more specific with more details now, please reconsider it. Commented Mar 2, 2015 at 8:18

2 Answers 2

Reset to default
2

Making "wrapper properties" like this is fine if it's the cleanest way to implement the API that you want this class to have.

However, the specific example you posted does look questionable to me, mostly because the user of your class can now do exactly the same thing in two different ways: myPage.MainParagraph.Text and myPage.MainText. Since the user doesn't (and shouldn't) know that one simply wraps the other, this raises all sorts of questions he shouldn't have to ask. Yes, documentation can help, but in my opinion the confusion and complication added by this duplicate outweighs the convenience of not having to type those extra two characters.

What you probably want to do is completely hide the TextBlock object, and provide wrappers for only those properties that you want the Page user to have access to. Then if you later decide TextBlock is not the best implementation of Page, it's much easier to change things. Alternatively, you can leave the TextBlock completely exposed with no wrappers, if you think that telling the user "this is how you get the TextBlock object that Page uses" is a better choice for your API.

Edit: Now that you've updated the question, and I can see that your Page class contains two TextBlock objects, I would lean very strongly towards making the two TextBlocks public and having no wrappers. Any attempt to wrap them immediately leads to potential confusion with disambiguating the MainParagraph wrapper from the Footer wrapper from the Header wrapper you'll probably add in version 3.0 from any of the actual TextBlock properties they're all implemented with. It's so much simpler to just expose both and let the TextBlock API do its job.

Improve this answer
2
  • I changed the example to say my real problem, please consider it. Commented Mar 2, 2015 at 8:18
  • Hm, I think my answer and @DixonD's are still valid; we just need to change the class names. But now I can say that making the TextBlocks public and having no wrappers would be the far more intuitive option. Commented Mar 2, 2015 at 8:23
3

I believe that you should either expose object TextBlock from Page or have delegate methods in Page to provide access to the methods of TextBlock but not both. I guess it depends how many properties and methods of TextBlock you want to expose, so if they are many, that just provide TextBlock and do not have all those simple delegations.

You can also check two opposite refactorings from the Fowler's catalog: Hide Delegate and Remove Middle Man

Improve this answer

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.