Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some parts like the router class is not swifty #22

Open
crashoverride777 opened this issue Sep 12, 2017 · 3 comments
Open

Some parts like the router class is not swifty #22

crashoverride777 opened this issue Sep 12, 2017 · 3 comments

Comments

@crashoverride777
Copy link

A lot of this architecture seems very unswifty and seems designed for older languages. For example no good swift developer would use the clunky Router class instead of a simple extension of the view controller.

@crashoverride777 crashoverride777 changed the title Clean architecture is not swifty Router is not swifty Sep 12, 2017
@crashoverride777 crashoverride777 changed the title Router is not swifty Some parts like the router class is not swifty Sep 12, 2017
@basememara
Copy link

More Swifty router implemented in PR #25. Instead of segues, use the extended show API with completion handler for configuring target view controller:

class ListOrdersRouter: NSObject, RouterProtocol,...
{
  func routeToShowOrder(for id: String)
  {
    show(storyboard: .showOrder) { (destinationVC: ShowOrderViewController) in
      let selectedOrder = self.dataStore?.orders?.first { $0.id == id }
      var destinationDS = destinationVC.router!.dataStore!
      self.passDataToShowOrder(for: selectedOrder, destination: &destinationDS)
    }
  }
  
  func routeToCreateOrder()
  {
    show(storyboard: .createOrder) { (destinationVC: CreateOrderViewController) in
      var destinationDS = destinationVC.router!.dataStore!
      self.passDataToCreateOrder(source: self.dataStore!, destination: &destinationDS)
    }
  }

Also, each scene has its own storyboard instead of placed all in one main storyboard:

capturfiles_16

Let me know what you think.

@crashoverride777
Copy link
Author

crashoverride777 commented Oct 24, 2017

Hey, the changes are very nice. Separate storyboards for each unique scene is the way to go. However the router class itself is still not swifty. Swift is a protocol and extension orientated language, so having a class with 2 methods and a weak property to the VC is clunky. It’s also a memory leaker if weak is forgot as the VC is also referencing the rooter class. That whole class for example could easily just be a extension of the view controller, optionally in a new swift file. I am just pointing this out as I feel clean architecture is outdated for Swift, especially they way most tutorials and the cleanSwift website do it. Thanks for your detailed reply and your awesome repo.

@basememara
Copy link

Cool thx for taking a look and the feedback. I definitely agree with all your points.

I have another repo called stateless_arch where all parts are value-type structs instead of classes. My thought is the architectural flow should be stateless and immutable, with the exception of the view controller of course. That should be safer, simpler to think about, and in the spirit of Swift philosophy. It’s a bit of work so still chipping away at it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants