robb / Cartography

A declarative Auto Layout DSL for Swift :iphone::triangular_ruler:
Other
7.34k stars 527 forks source link

Remove `constrain` closures #145

Open robb opened 9 years ago

robb commented 9 years ago

As per @smileyborg in #144:

BTW, on the note of significant breaking changes like this, I'd love to also explore what Cartography would look like if it didn't use closures passed to constraint (or layout) and instead added some "NSLayoutAnchor-like" properties of type LayoutProxy to UIView/NSView, which would let you use the custom operators to create constraints without having to pass the view(s) into a closure first.

I've thought about this, we'd basically replace

constrain(view1, view2) {
    view1.width  == view2.width
    view1.height == view2.height
}

with, if I understood you right, something like

view1.car_width  == view2.car_width
view1.car_height == view2.car_height

There's two aspects I dislike about this approach. One is that I believe the car_ prefix makes it harder to read. I'd like Cartography to be as close to "pseudocode" as possible, and prefix takes away from that.
There may be another way to attach the properties, such as a surrounding function or an operator but I'm worried it takes away clarity.

The other thing is that I like that the constrain function forces users to have their constraints grouped in a single place, and also sections off the part of the code in which Cartography's operator overloading applies.

It also provides a place to attach additional information. E.g. I'd like to add __LINE__ and __FILE__ default arguments to constrain to make it easier to find where a specific constraint was created in code (#97).

smileyborg commented 9 years ago

Yeah, that's the general idea of how it would work. Except the main difference in what I was thinking is that we do not need any prefix (car_ in your example) for the methods added to UIView/NSView. There are two reasons why I think a prefix is not necessary:

  1. Swift has proper namespacing so that two methods of the same name don't actually collide and cause undefined behavior at runtime, like Objective-C. So if Cartography defines some methods in an extension that even UIKit decides to adopt in some future SDK version, the implementations remain separate (and if you had to, you can explicitly call one based on its module). Disclaimer: I've not tried this myself yet.
  2. Even with point 1 above, I think it's completely reasonable to come up with a slightly more unique name than just width, height, etc -- very much like NSLayoutAnchor names: widthAnchor, heightAnchor, etc. What about layoutWidth, layoutHeight, etc? Or we could even just hang 1 property on UIView/NSView: layout, and then expose each attribute without a prefix on that one property. So it would look like:
view1.layout.width == view2.layout.width
view1.layout.height == view2.layout.height

Personally, I definitely lean towards eliminating the need to call a function and especially the need to pass in everything you want to constrain up front, when clearly it's not actually necessary. This would remove code from and simplify Cartography itself (c.f. ec439ab6e33b3fd0c5b94b03d4157e91e7cc0ef3) as well. I think it would be a lot cleaner, and would be worth the tradeoffs.

I don't think it's our responsibility to force users of the library to organize their code in a certain way, so I don't buy that justification of using a constrain function. For the layout function it made sense, because it was creating a scope at the end of which we'd run layoutIfNeeded, but that function was just removed.

And Cartography's operator overloading is very reasonable because it only applies to the LayoutProxy type. So I don't find it "risky" to have it available globally.

And to #97, this should be completely doable with either approach. (I considered doing this for PureLayout in fact, just never got around to it!)

Ultimately I'd love to see this implemented in a separate branch, to compare against the current way of doing things. It should be fairly easy. I think that's the best way to truly judge the merits of each approach, and other users of Cartography would be able to weigh in.

zwaldowski commented 9 years ago

I will speak to really liking the constrain scopes, and regardless of it "actually" being a problem in practice or not, having foo == bar in a bare scope reads like an unused equality statement and not anything even remotely related to layout.

AliSoftware commented 9 years ago

I'd personally lean to a syntax like viewA.layout.left = 0.3 * viewB.layout.height + 10, feels more natural.

The constrain scope is nice, but would need to pass the views ahead of time which could maybe not always match the constraints of the code and context, so I'd agree with @smileyborg on that. Let the user add some code formatting to organize its code and constraints himself if he wants to but not impose it.