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

Code clean up #2

Closed
wants to merge 1 commit into from
Closed

Code clean up #2

wants to merge 1 commit into from

Conversation

amadeu01
Copy link
Contributor

@amadeu01 amadeu01 commented Feb 2, 2019

I removed the reposViewModel variable from:

  • func reposViewModel(_ reposViewModel: ReposViewModel, isLoading: Bool)
  • func reposViewModel(_ reposViewModel: ReposViewModel, didReceiveRepos repos:)
  • func reposViewModel(_ reposViewModel: ReposViewModel, didSelectId id: Int)

Delegate methods

@amadeu01
Copy link
Contributor Author

amadeu01 commented Feb 2, 2019

@tailec this implementation of the MVVM is really close to the MVP. I cannot tell the difference between this implementation and MVP.

@tailec
Copy link
Owner

tailec commented Feb 4, 2019

I've added self to delegate methods because I was following apple delegation pattern like:
func tableView(UITableView, didSelectRowAt: IndexPath)
Found some explanation there
https://developer.apple.com/library/archive/documentation/General/Conceptual/DevPedia-CocoaCore/Delegation.html

About MVP:
I think in MVP, presenter can have access to view (imports UIKit) but view models in MVVM are forbidden to do that.

@amadeu01
Copy link
Contributor Author

amadeu01 commented Feb 4, 2019

MVP just create a layer of abstraction to the view. So, the view communicate to the presenter by the protocol. But, the presenter should not have references to the UIKit. The ideia is to be able to test the view without a device (simulator). Like run the tests on macOS

@amadeu01
Copy link
Contributor Author

I'm gonna close it

@amadeu01 amadeu01 closed this Feb 15, 2019
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

Successfully merging this pull request may close these issues.

None yet

2 participants