Skip to content
This repository has been archived by the owner on Jan 20, 2024. It is now read-only.

Card type detection fixed #19

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

nagarro-mohitpadalia
Copy link

Hi @maxkramer

The PR fixes the issue #18

The caseIterable protocol works for enumerations without associated values, so I have added a simple solution to get the count of all enum cases:

Please review.

@maxkramer
Copy link
Owner

@nagarro-mohitpadalia thanks a lot for your support! That's a good find, and I'm surprised that the CaseIterable doesn't support enum int associated values, as enums are usually integer based already.

I think it would probably be better to use something along the lines of the following to ensure that we don't have an issue in the future with adding additional cases and forgetting to update the count variable. This loops through 1..n to calculate all cases. What are your thoughts?

https://gist.githubusercontent.com/polac24/f33791151e22d4787a09262d66f01bcd/raw/8ea9e37fae685fb9f21fbbbfaa601e51f2b927af/IntEnum.swift

Stolen from: https://medium.com/@londeix/listing-all-cases-in-an-enum-3b057f2c1432

@nagarro-mohitpadalia
Copy link
Author

@maxkramer

I have done the suggested change you can review and merge.

@maxkramer
Copy link
Owner

@nagarro-mohitpadalia that's great, thanks a lot for your support! Just waiting for CI, and will then get that merged!

@maxkramer
Copy link
Owner

maxkramer commented Jan 7, 2019

@nagarro-mohitpadalia circleci have removed our free plan for Mac OSX builds, currently waiting for them to renew it. Will merge thereafter.

@maxkramer
Copy link
Owner

@nagarro-mohitpadalia CircleCI have now fixed the issue. Please could you push the commit again / push a blank commit to trigger the CI build?

@maxkramer maxkramer self-requested a review January 7, 2019 23:15
@nagarro-mohitpadalia
Copy link
Author

@maxkramer I have pushed dummy commits, hope this will trigger the CI build

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

Successfully merging this pull request may close these issues.

3 participants