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

Operator Overloading downsides #2

Closed
robertjpayne opened this issue Jun 7, 2014 · 17 comments
Closed

Operator Overloading downsides #2

robertjpayne opened this issue Jun 7, 2014 · 17 comments

Comments

@robertjpayne
Copy link
Member

I'm working just locally at the moment but brought a similar fork of mine up to speed and actually working, in the progress I've come across some unfortunate quirks for operator overloading:

To rework something like (v1.left.top.right == v2) + UIEdgeInsets(5, 5, 5, 5) to v1.left.top.right == v2 + UIEdgeInsets(5, 5, 5, 5) we have to operator overload + on UIView which starts to get a bit unfortunate.

Priorities typically would always be set last using the Objective-C DSL but with the new DSL we must specify the priority like first like v1.left == v2.priority(500) + 50 or use parans like (v1.left == v2 + 50).priority(500). The only other option for this to make it nice and clean would be using a different operator just for priorities. I'm currently using ~ so it looks like v1.left == v2 + 15 ~ 749 but we really start to lose code readability going this far.

@robertjpayne robertjpayne changed the title Exploring new API/DSL Direction Operator Overloading downsides Jun 7, 2014
@robertjpayne
Copy link
Member Author

The other disadvantage here is if we want to maintain a unified codebase that works with existing Objective-C projects we'll need to write the "makeConstraints" API and allow DSL usage anyways because Objective-C cannot call Swift API's that use features like operator overloading or generics.

@robertjpayne
Copy link
Member Author

Personally I just feel like the old DSL v1.left.equalTo(v2).offset(15).priority(749) would still work perfectly fine and actually lends itself even better to Swift due to the syntax similarities.

Since this is all for layout code readability is extremely important and the old DSL clearly spelled out every step of the way what it was doing, this new one could potentially be a bit hairy coming back to old projects with loads of layout code.

@cloudkite
Copy link
Contributor

@robertjpayne good points, thanks for investigating! It would be very handy to be able to copy code inside a makeConstraints line for line from a objc project over to a swift project.

I think a unified API is possible by keeping the code bases separate. I'm not sure a unified codebase is possible given that if we convert the existing code to swift, any normal func will need to be invoked with square brackets from objc right?

// swift
make.left.equalTo(view1).offset(15)

// when calling swift version of masonry in objc
[[make.left equalTo:view1] offset:15]

This could cause issues for projects mixing objc and swift
Also how did you get the UIView category to compile?

@ahti
Copy link

ahti commented Jun 7, 2014

@cloudkite We could (in theory) still have properties that hold lambdas in swift, which would translate to blocks in obj-c (as far as i know)

To rework something like (v1.left.top.right == v2) + UIEdgeInsets(5, 5, 5, 5) to v1.left.top.right == v2 + UIEdgeInsets(5, 5, 5, 5) we have to operator overload + on UIView which starts to get a bit unfortunate.

This should be solvable by using a custom operator with a higher binding precedence than +, etc.

And we'd only be overloading +(left: View, right: CGFloat) and +(left: View, right: UIEdgeInsets) anyway, so I think it wouldn't be that bad.

I'll also try to work something out later today and see how it goes ^^

@cloudkite
Copy link
Contributor

@ahti so something like

class ViewConstraint {
    var equalTo : (Any) -> ViewConstraint {
        get {
            return { (Any) -> (ViewConstraint) in return self }
        }
    }
}

var left = ViewConstraint();
left.equalTo(1);

Would be interesting to call it from objc and see if its equivalent

@cloudkite
Copy link
Contributor

@robertjpayne for simple constraints like view1.left == view2.right + 10 operator overloading is very elegant and readable.
However I agree that for more complicated constraints the existing chaining API might be more straightforward and readable. Especially once you start adding more than one operator into a constraint equation and you need to start worrying about operator precedence.

@robertjpayne
Copy link
Member Author

@ahti @cloudkite

Yea overloading more than one operator starts to get hairy, I don't particularly like overloading the + operator on UIView either but there's not really any way around that.

I'd say it'll also be easier for users to migrate if we keep the chaining DSL identical and allow overloading for those that maybe want even more shorthand form? I use priorities quite a lot and to me I want to keep all my layout code the same and not mix between operators and chaining styles. I guess the problem is the idea of "elegant and readable" starts to get thrown out once we have to mix and match like:

installConstraints(
  view1.left == view2.right + 10
  view1.top.equalTo(view2).priority(500)
  view1.width == 50
  view2.top == view3.bottom - 50
)

Maybe only using operator overloads in the equalTo / lessThanOrEqualTo / greaterThanOrEqualTo could solve it?

installConstraints(
  view1.left.equalTo(view2 + 50)
  view1.top.equalTo(view2).priority(500)
  view1.width.equalTo(50)
  view2.top.equalTo(view3.bottom - 50)
)

As for getting the extension to work, I've moved all of the left/right/top/bottom etc... attributes into a "Layout" class and added a single "layout" property to UIView. I expose that "layout" property through a standard Objective-C category so that the compiler is happy. Once the compiler gets fixed these can easily move back to UIView. Part of me actually likes it nested in it's own property to avoid collisions.

@nickynick
Copy link

@robertjpayne Have you got a working prototype? I took yesterday to build mine, but in the very end I encountered a compiler bug. It looks exactly as in this gist, and basically it prevents usage of computed properties like view1.left. I wonder if there is a workaround for now?

@robertjpayne
Copy link
Member Author

@nickynick Nah it's still busted, I've been holding off on any swift development until the tools stabilise a bit more. Been busy anyways!

@nickynick
Copy link

@robertjpayne Yeah, makes sense. The thing has been crashing and bugging wildly on me, it's a bummer. Anyway, I think I gonna workaround this somehow and open a PR so you guys can toy with it a bit.

@robertjpayne
Copy link
Member Author

@nickynick I should say I did have a workaround but it wasn't pretty, you basically cannot create extensions on an Objective-C class, you must Subclass to add functionality.

So instead I made a "Layout" class and in Objective-C added @property Layout *layout to UIView

All in all it was just wasn't worth it. I'm betting DP3 is going to have some nice improvements to Swift, specifically method scoping and then doing some real development may be worthwhile.

@nickynick
Copy link

@robertjpayne Oh, I see, thanks! That's right, it should work this way.

From my testing, this bug is not really limited to Objective-C classes, it occurs for any class. E.g.

class Foo {
}

extension Foo {
    var bar: Int { return 42 }
}

let foo = Foo()
foo.bar // kaboom

@cloudkite
Copy link
Contributor

btw have you guys seen this https://github.com/robb/Cartography? Seeing as operator overloading has already been done. I'm thinking of using this repository to make a Swift port of Masonry rather than a operator overloading DSL.

If anyone wants to jump in and make a start, feel free. I don't think I will have spare time anytime soon :(

@nickynick
Copy link

I've put together Tails: https://github.com/nickynick/Tails. It borrows many great ideas from Masonry, so it may feel familiar. Feel free to check it out :)

@cloudkite
Copy link
Contributor

Awesome looks great!

On Thu, Jul 3, 2014 at 12:25 AM, Nick Tymchenko notifications@github.com
wrote:

I've put together Tails: https://github.com/nickynick/Tails. It borrows many great ideas from Masonry, so it may feel familiar. Feel free to check it out :)

Reply to this email directly or view it on GitHub:
#2 (comment)

@nickynick
Copy link

I hope I'll get my hands on Masonry port sooner than later as well. Masonry style DSL has its charm too and would be pretty straightforward to implement in Swift.

@robertjpayne
Copy link
Member Author

Going to close this issue in favour of #1 for any further operator overload discussions. For now Snappy doesn't support any.

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

4 participants