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

Prevent duplicate constraints. #49

Closed
davidbjames opened this issue Oct 15, 2017 · 5 comments
Closed

Prevent duplicate constraints. #49

davidbjames opened this issue Oct 15, 2017 · 5 comments

Comments

@davidbjames
Copy link

Adding constraints should probably prevent duplicates.

From basic testing, duplicate constraints do not appear to cause technical problems. However, if those constraints are iterated/manipulated it could cause problems.

ConstraintBatch would need to perform a "union" instead of "append". (See here.) Unfortunately, even though NSLayoutConstraint is Hashable, apparently duplicate constraints still have unique hash values, so Set can't be used to remove duplicates. You would need to do an Equatable based "union" instead. I have some code for that here.

@ZevEisenberg
Copy link
Collaborator

It's typically the developer's responsibility to avoid creating duplicate constraints, but this could be an interesting way to help avoid developer error. Are you running into situations where you're creating duplicate constraints when you don't mean to? I'm wondering if there's anything we can do in terms of the API to make this less likely in the first place.

@davidbjames
Copy link
Author

I'm building a library that supports declarative layouts. There is nothing preventing a developer from inadvertently adding duplicate constraints. This conflicts with another part of my API that supports changing constraints constants, which involves querying the UIView.constraints property. The problem, is if there are duplicate constraints, then it throws off querying the constraints.

@davidbjames
Copy link
Author

Just realized ConstraintBatch.constraints is not per layout object, but per instance of itself, so "union'ing" there would be pointless.

In answer to your question about making it less likely to occur, I personally wouldn't pollute the API (it's already excellent). It's not that critical.

The only thing for it is to preemptively check for duplicates before each is added, but I'm beginning to wonder if Apple had a reason to allow duplicate constraints to be set in the first place.

@ZevEisenberg
Copy link
Collaborator

I can think of a pathological case where you'd want duplicate constraints: you add two identical pinned width constraints with a low priority, then later you change the .constant of one of them to a different value. That's a legitimate use case, and you wouldn't want to be warned about it.

@jvisenti
Copy link
Contributor

Agree with @ZevEisenberg that's it's the developer's responsibility to decide what constraints to add, and to avoid duplicates. Closing for now.

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

3 participants