Skip to content

Remove unnecessary explicit conformance to Equatable #30

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sergeysmagleev
Copy link

I noticed that the models in Order.swift explicitly implement the == function to conform to Equatable. This means that each time a model is updated, this method must be updated too, which makes it easy to make a mistake by forgetting to do so. This could be avoided by making all properties inside the models conform to Equatable too.
Also, lhs.date.timeIntervalSince(rhs.date) < 1.0 comparison wasn't working correctly because for each case when rhs is later than lhs it would always return true.
This also revealed an error in the Unit Tests when OrdersWorker.updateOrder method wasn't returning the correct order with the updated date in it.
Therefore I have:

  • Removed all instances of explicitly implemented == method
  • Declared necessary Model structs as conforming to Equatable
  • Made updateOrder method inside OrdersWorkerTests test suite return the Order object passed to it inside the completionHandler instead of the static mock object from OrdersWorkerTests.testOrders

Copy link

@Pratik-Somaiya Pratik-Somaiya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

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.

2 participants