Skip to content

Add setStringValue:forExpression method on UIView#186

Open
cadrega wants to merge 2 commits intonicklockwood:developfrom
cadrega:uiview-setstringvalue-forexpression
Open

Add setStringValue:forExpression method on UIView#186
cadrega wants to merge 2 commits intonicklockwood:developfrom
cadrega:uiview-setstringvalue-forexpression

Conversation

@cadrega
Copy link

@cadrega cadrega commented Sep 26, 2019

The setStringValue:forExpression method on UIView can set any value from a string, just like Layout does when looking at the XML attributes for a View. This expands the possibilities of the setValue:forExpression, which needs the value to be of the right type.

The setStringValue:forExpression method on UIView can set any value from a string, just like Layout does when looking at the XML attributes for a View. This expands the possibilities of the setValue:forExpression, which needs the value to be of the right type.
Copy link
Owner

@nicklockwood nicklockwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few style issues with the code, but otherwise seems reasonable.

if let typ = type(of: self).cachedExpressionTypes[name] {
if let node = _layoutNode {
if let exp = LayoutExpression(expression: value, type: typ, for:node) {
if let res = try? exp.evaluate() {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The setStringValue is marked as throwing, but all the errors are suppressed using try? - was that intentional?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initially copied the method signature of the other setValue, and so kept the throwing attribute.
For my project, I didn't really care about any error - set it or fail silently.
For the general use case, maybe it should be best for the errors to bubble up. (Although I don't know what can be done with the error, outside of this class... Maybe logging it?)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I thought about it and it should be good to bubble up errors that happen during expression evaluation and newvalue setting.

Fix indenting style (spaces, not tabs)
Remove the "if pyramid of doom" by combining all let statements into a single guard
Don't suppress errors while evaluating the expression or setting the value
@cadrega
Copy link
Author

cadrega commented Sep 27, 2019

I updated the branch, addressing your comments. Is that the right thing to do, or should I create a new pull request?

@cadrega
Copy link
Author

cadrega commented Oct 2, 2019

Using this mod on Layout, I noticed two things:

  • I tried to set a value on a UITabBarController, but obviously it didn't work, because it's not a subclass of UIView
  • I tried to set a "width" value, but it's not working. The same happened using "left", but "center.x" it's working well. And again, setting "center.x" to "40%" it's not working, it only works with an absolute value.

By "not working" I mean the method has been called but no visible effect was achieved, and no error was spit out.

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.

2 participants