-
Notifications
You must be signed in to change notification settings - Fork 429
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
Add possibility to use and
+ or
with collection of signals/properties
#735
Conversation
Sources/FoundationExtensions.swift
Outdated
internal func and() -> Bool { | ||
return reduce(true) { $0 && $1 } | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer not to add these extensions. They pollute the namespace and aren't used widely enough to justify that—nor do they add that much over .reduce(true, &&)
.
Sources/Property.swift
Outdated
/// - property: Collection of properties to be combined. | ||
/// | ||
/// - returns: A property that contains the logial AND results. | ||
public static func and<P: PropertyProtocol, Properties: Collection>(_ properties: Properties) -> Property<Value> where P.Value == Value, Properties.Element == P { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all
seems like a better name for this
Sources/Property.swift
Outdated
/// - properties: Collection of properties to be combined. | ||
/// | ||
/// - returns: A property that contains the logial OR results. | ||
public static func or<P: PropertyProtocol, Properties: Collection>(_ properties: Properties) -> Property<Value> where P.Value == Value, Properties.Element == P { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any
seems like a better name for this
Thanks for the review, all issues should be resolved now. |
1. add possibility to use `and` and `or` operators with array of arguments (#735, kudos to @olejnjak) | ||
```swift | ||
let property = Property.or([boolProperty1, boolProperty2, boolProperty3]) | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to be updated now
Looks good to me. I'll leave this open for a day or two in case anyone else has feedback. |
Thanks! Sorry for the delay. |
Usually on my projects I do use more properties to determine state of some
Property
, e.g.isLoading
property usually depends on more properties which later on ends like thisThen I end up with extensions which simplify such code and I think it might be useful for more people than just me.
This PR adds static function
and
andor
toSignalProducer
,PropertyProtocol
with appropriate attributes which solves such case elegantly.Checklist