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

Make Edit a type of a Collection #40

Merged
merged 2 commits into from
Jul 27, 2017
Merged

Make Edit a type of a Collection #40

merged 2 commits into from
Jul 27, 2017

Conversation

osteslag
Copy link
Owner

This set of changes is about making Edit a type of a collection instead of a collection’s equatable elements plus the resulting changed requirements, including tests and test app. Merges into the version 3 branch.

An Edit needs to have a concept of the collection type so we can ensure we can do arithmetic on the collection’s index distance type, see discussion on #39.

I wasn’t able to declare …where C.IndexDistance: SignedInteger on Edit, Changeset while doing enumerations and additions/subtractions on offsets in a compatible way. At least I couldn’t figure out how to. So for now, we require Int index distances on the collections.

Note: I prefer to ensure all commits are compilable (git-bisect), hence all files in one commit.

FYI: I’m planning a general cleanup for version 3 (including using the C shorthand for Collection types, probably splitting up Edit and Changeset in separate files).

An Edit needs to have a concept of the collection type so we can ensure
we can do arithmetic on the collections index distance type.
@osteslag osteslag self-assigned this Jul 26, 2017
@osteslag osteslag requested a review from ole July 26, 2017 19:22
@ole
Copy link
Contributor

ole commented Jul 26, 2017

That was fast! I'll take a look at it soon.

@osteslag
Copy link
Owner Author

I have a full-time job, so I could have been much faster 😉
But compared to the previous PR, based on your comment in April, I guess you’re right 😆

@ole
Copy link
Contributor

ole commented Jul 27, 2017

@osteslag I haven't reviewed the changes yet, but I spent some time on getting rid of the IndexDistance == Int constraint. See #41.

The T: Collection constraint is redundant because T is already
constrained to Collection through Edit<T> and Swift can infer this.

While the explicit constraint arguably makes some method signatures
clearer, I removed them because Xcode 9 generated warnings.
@ole
Copy link
Contributor

ole commented Jul 27, 2017

I pushed one commit removing some redundant constraint. Xcode 9 detected them for me. Xcode 8 doesn't show the warnings, but they are still redundant; the project compiles and runs without them just fine.

screen shot 2017-07-27 at 15 20 11

Everything else looks good to me. 👍

@osteslag
Copy link
Owner Author

Good change! Clearly Collection as well as the other Edit collection constraints are inferred via Edit<T>.

@osteslag osteslag merged commit 550e4bd into version-3 Jul 27, 2017
@osteslag osteslag deleted the edit-of-collection branch July 27, 2017 13:26
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