hyperoslo / Pages

:page_facing_up: UIPageViewController made simple
http://hyper.no
Other
495 stars 51 forks source link

Subclassing #16

Closed vadymmarkov closed 9 years ago

vadymmarkov commented 9 years ago

PagesController is a subclass of UIPageViewController and we make the things I don't really like: it's delegate and dataSource of self. I think it's better approach to make it a subclass of UIViewController and have a property of UIPageViewController. What do you guys think @zenangst @3lvis @kostiakoval ?

kostiakoval commented 9 years ago

I usually prefer composition over inheritance. For datasource I like having separate component Example TableViewArrayDataSource, it can be reused in many VC than.

3lvis commented 9 years ago

I think subclassing makes it easier to understand that it relays heavily on UIPageViewController, or that is a child of UIPageViewController.

For me having a property of UIPageViewController would feel like when people have a UITableView inside a subclass of UIViewController instead of just subclassing UITableViewController.

kostiakoval commented 9 years ago

The problem is that you want to provide customer controllers fordelegate and dataSource. Am i right?

vadymmarkov commented 9 years ago

PagesController implements delegate methods of UIPageViewController, but also we need a custom delegate of PagesController, so we have pagesDelegate and I don't like this, because it makes things a bit messy.

vadymmarkov commented 9 years ago

Also if you assign some class as a delegate of PagesController it will break everything. I think that we shouldn't allow to do that and it's better to hide everything that is not needed to be used.

3lvis commented 9 years ago

@vadymmarkov So the problem is the delegate naming, right?

You would like to just use delegate instead of pagesDelegate. This isn't really related to subclassing that's just one way to fix it.

vadymmarkov commented 9 years ago

It's one side of the problem, the other one is that we provide custom implementation of UIPageViewController, so I would like to hide all the logic and provide a public interface that allows you to deal only with PagesController, not with UIPageViewController. I see your point that maybe subclassing makes it easier to understand that it relays heavily on UIPageViewController. But usually we use UIPageViewController without subclassing, please correct me if I'm wrong. UITableViewController deals with UITableView, in our case I think that PagesController have to deal with UIPageViewController.

kostiakoval commented 9 years ago

That's a great point, I agree with @vadymmarkov. Now you can change a delegate of a UIPageViewController and PagesController wont work than.

it would be better to hide all implementation details

3lvis commented 9 years ago

Current Generated API:

screen shot 2015-05-18 at 11 46 44

vadymmarkov commented 9 years ago

But it doesn't mean that you can't use public properties or methods extended from UIPageViewController.

vadymmarkov commented 9 years ago

I'm still on removing subclassing here :smile: What do you think @zenangst ?

zenangst commented 9 years ago

I agree with @vadymmarkov :+1:

vadymmarkov commented 9 years ago

I forgot about the things that we need to extend from UIPageViewController, like transitionStyle, navigationOrientation, pageControl. If we are going with wrapper solution we have to duplicate a lot of code and it's not better than clean API. So I think I close it for now.