ronantakizawa/codereview-bench huggingface dataset:
Repository: ReactiveX/RxSwift
File: RxCocoa/iOS/UIDatePicker+Rx.swift (Swift)

Code before:
//
//  UIDatePicker+Rx.swift
//  RxCocoa
//
//  Created by Daniel Tartaglia on 5/31/15.
//  Copyright © 2015 Krunoslav Zaher. All rights reserved.
//

#if os(iOS)

#if !RX_NO_MODULE
import RxSwift
#endif
import UIKit

extension Reactive where Base: UIDatePicker {
    /// Reactive wrapper for `date` property.
    public var date: ControlProperty<Date> {
        return value
    }

    /// Reactive wrapper for `countDownDuration` property.
    public var countDownDuration: ControlProperty<TimeInterval> {
        return countDownValue
    }
    
    /// Reactive wrapper for `date` property.
    public var value: ControlProperty<Date> {
        return UIControl.rx.value(
            self.base,
            getter: { datePicker in
                datePicker.date
            }, setter: { datePicker, value in
                datePicker.date = value
            }
        )
    }

    /// Reactive wrapper for `countDownDuration` property.
    public var countDownValue: ControlProperty<TimeInterval> {
        return UIControl.rx.value(
            self.base,
            getter: { datePicker in
                datePicker.countDownDuration
            }, setter: { datePicker, value in
                datePicker.countDownDuration = value
            }
        )
    }
}

#endif


Review comment (refactor):
Hi @pawurb ,

I'm not sure what would be the proper naming conventions for this case. Let's only add `countDownDuration` property for now and just move this implementation there.

Code after:
//
//  UIDatePicker+Rx.swift
//  RxCocoa
//
//  Created by Daniel Tartaglia on 5/31/15.
//  Copyright © 2015 Krunoslav Zaher. All rights reserved.
//

#if os(iOS)

#if !RX_NO_MODULE
import RxSwift
#endif
import UIKit

extension Reactive where Base: UIDatePicker {
    /// Reactive wrapper for `date` property.
    public var date: ControlProperty<Date> {
        return value
    }

    /// Reactive wrapper for `date` property.
    public var value: ControlProperty<Date> {
        return UIControl.rx.value(
            self.base,
            getter: { datePicker in
                datePicker.date
            }, setter: { datePicker, value in
                datePicker.date = value
            }
        )
    }

    /// Reactive wrapper for `countDownDuration` property.
    public var countDownDuration: ControlProperty<TimeInterval> {
        return UIControl.rx.value(
            self.base,
            getter: { datePicker in
                datePicker.countDownDuration
            }, setter: { datePicker, value in
                datePicker.countDownDuration = value
            }
        )
    }
}

#endif
Repository: ReactiveX/RxSwift
File: Tests/RxCocoaTests/PublisherTests.swift (Swift)

Code before:
        }
        
        scheduler.scheduleAt(285) {
            disposable3.dispose()
        }
        
        scheduler.start()
        
        XCTAssertEqual(observer1.events, [
            next(210, 1),
            next(225, 2)
            ])
        
        XCTAssertEqual(observer2.events, [
            next(225, 2),
            next(245, 3)
            ])
        
        XCTAssertEqual(observer3.events, [
            next(265, 4)
            ])
    }
}

extension PublisherTest {
    func testEventHubAlwaysObservingOnMainThread() {
        var observedOnMainQueue = false
        
        let expectSubscribeOffMainQueue = expectation(description: "Did subscribe off main thread")
        
        let publisher = Publisher<Int>()
        
        _ = publisher.asEventHub().asObservable().subscribe(onNext: { value in
            XCTAssertTrue(DispatchQueue.isMain)
            observedOnMainQueue = true
            XCTAssertEqual(value, 1)
        })
        
        doOnBackgroundQueue {
            let d = publisher.asObservable().subscribe { n in
                
            }
            let d2 = publisher.asObservable().subscribe { n in
                
            }
            doOnMainQueue {
                d.dispose()
                d2.dispose()
                expectSubscribeOffMainQueue.fulfill()
            }
        }

Review comment (suggestion):
I don't think this unit tests tests anything useful. 

The logic should be:
* subscribe
* send element on background thread to `publisher`
* make sure element is received on main thread and fulfill expectation

Code after:
        }
        
        scheduler.scheduleAt(285) {
            disposable3.dispose()
        }
        
        scheduler.start()
        
        XCTAssertEqual(observer1.events, [
            next(210, 1),
            next(225, 2)
            ])
        
        XCTAssertEqual(observer2.events, [
            next(225, 2),
            next(245, 3)
            ])
        
        XCTAssertEqual(observer3.events, [
            next(265, 4)
            ])
    }
}

extension PublishRelayTest {
    func testPublisherAlwaysObservingOnMainThread() {
        
        let relay = PublishRelay<Void>()
        let mainThreadExpectation = expectation(description: "PublishRelay emits items on main thread")
        
        let d = relay.asObservable().subscribe(onNext: {
            XCTAssertTrue(DispatchQueue.isMain)
            mainThreadExpectation.fulfill()
        })
        
        doOnBackgroundQueue {
            relay.accept(())
        }
        
        waitForExpectations(timeout: 0.5, handler: nil)
        d.dispose()
    }
}
Repository: ReactiveX/RxSwift
File: RxExample/RxExample/Examples/APIWrappers/APIWrappersViewController.swift (Swift)

Code before:
        sliderValue.asObservable()
            .subscribe(onNext: { [weak self] x in
                self?.debug("UISlider value \(x)")
            })
            .disposed(by: disposeBag)


        // MARK: UIDatePicker

        // also test two way binding
        let dateValue = Variable(Date(timeIntervalSince1970: 0))
        _ = datePicker.rx.date <-> dateValue


        dateValue.asObservable()
            .subscribe(onNext: { [weak self] x in
                self?.debug("UIDatePicker date \(x)")
            })
            .disposed(by: disposeBag)


        // MARK: UITextField

        // also test two way binding
        let textValue = Variable("")
        (textField.rx.textInput <-> textValue).disposed(by: disposeBag)

        textValue.asObservable()
            .subscribe(onNext: { [weak self] x in
                self?.debug("UITextField text \(x)")
            })
            .disposed(by: disposeBag)


        // MARK: UIGestureRecognizer

        mypan.rx.event
            .subscribe(onNext: { [weak self] x in
                self?.debug("UIGestureRecognizer event \(x.state)")
            })
            .disposed(by: disposeBag)


        // MARK: UITextView

        // also test two way binding
        let textViewValue = Variable("")
        (textView.rx.textInput <-> textViewValue).disposed(by: disposeBag)

        textViewValue.asObservable()
            .subscribe(onNext: { [weak self] x in

Review comment (suggestion):
Hi @carlosypunto ,

We don't need `.disposed(by: disposeBag)` in this case and on line 164. I'm using this code to test will `<->` work without dispose bags.

Code after:
        sliderValue.asObservable()
            .subscribe(onNext: { [weak self] x in
                self?.debug("UISlider value \(x)")
            })
            .disposed(by: disposeBag)


        // MARK: UIDatePicker

        // also test two way binding
        let dateValue = Variable(Date(timeIntervalSince1970: 0))
        _ = datePicker.rx.date <-> dateValue


        dateValue.asObservable()
            .subscribe(onNext: { [weak self] x in
                self?.debug("UIDatePicker date \(x)")
            })
            .disposed(by: disposeBag)


        // MARK: UITextField

        // also test two way binding
        let textValue = Variable("")
        _ = textField.rx.textInput <-> textValue

        textValue.asObservable()
            .subscribe(onNext: { [weak self] x in
                self?.debug("UITextField text \(x)")
            })
            .disposed(by: disposeBag)


        // MARK: UIGestureRecognizer

        mypan.rx.event
            .subscribe(onNext: { [weak self] x in
                self?.debug("UIGestureRecognizer event \(x.state)")
            })
            .disposed(by: disposeBag)


        // MARK: UITextView

        // also test two way binding
        let textViewValue = Variable("")
        _ = textView.rx.textInput <-> textViewValue

        textViewValue.asObservable()
            .subscribe(onNext: { [weak self] x in
Repository: ReactiveX/RxSwift
File: RxExample/Extensions/RxCLLocationManagerDelegateProxy.swift (Swift)

Code before:
    import RxCocoa
#endif

class RxCLLocationManagerDelegateProxy : DelegateProxy
                                       , CLLocationManagerDelegate
                                       , DelegateProxyType {

    internal lazy var didUpdateLocationsSubject = PublishSubject<[CLLocation]>()
    internal lazy var didFailWithErrorSubject = PublishSubject<Error>()

    class func currentDelegateFor(_ object: AnyObject) -> AnyObject? {
        let locationManager: CLLocationManager = object as! CLLocationManager
        return locationManager.delegate
    }

    class func setCurrentDelegate(_ delegate: AnyObject?, toObject object: AnyObject) {
        let locationManager: CLLocationManager = object as! CLLocationManager
        if let delegate = delegate {
            locationManager.delegate = (delegate as! CLLocationManagerDelegate)
        } else {
            locationManager.delegate = nil
        }
    }

    public func locationManager(_ manager: CLLocationManager, didUpdateLocations locations: [CLLocation]) {
        didUpdateLocationsSubject.onNext(locations)
    }

    public func locationManager(_ manager: CLLocationManager, didFailWithError error: Error) {
        didFailWithErrorSubject.onNext(error)
    }
}


Review comment (suggestion):
This needs to forward call to `_forwardToDelegate?.locationManager(manager, didUpdateLocations: locations)`

Code after:
    import RxCocoa
#endif

class RxCLLocationManagerDelegateProxy : DelegateProxy
                                       , CLLocationManagerDelegate
                                       , DelegateProxyType {

    internal lazy var didUpdateLocationsSubject = PublishSubject<[CLLocation]>()
    internal lazy var didFailWithErrorSubject = PublishSubject<Error>()

    class func currentDelegateFor(_ object: AnyObject) -> AnyObject? {
        let locationManager: CLLocationManager = object as! CLLocationManager
        return locationManager.delegate
    }

    class func setCurrentDelegate(_ delegate: AnyObject?, toObject object: AnyObject) {
        let locationManager: CLLocationManager = object as! CLLocationManager
        if let delegate = delegate {
            locationManager.delegate = (delegate as! CLLocationManagerDelegate)
        } else {
            locationManager.delegate = nil
        }
    }

    public func locationManager(_ manager: CLLocationManager, didUpdateLocations locations: [CLLocation]) {
        _forwardToDelegate?.locationManager(manager, didUpdateLocations: locations)
        didUpdateLocationsSubject.onNext(locations)
    }

    public func locationManager(_ manager: CLLocationManager, didFailWithError error: Error) {
        _forwardToDelegate?.locationManager(manager, didFailWithError: error)
        didFailWithErrorSubject.onNext(error)
    }
}
Repository: ReactiveX/RxSwift
File: Rx.playground/Pages/Playground.xcplaygroundpage/Contents.swift (Swift)

Code before:
/*:
 > # IMPORTANT: To use **Rx.playground**:
 1. Open **Rx.xcworkspace**.
 1. Build the **RxSwift-macOS** scheme (**Product** → **Build**).
 1. Open **Rx** playground in the **Project navigator**.
 1. Show the Debug Area (**View** → **Debug Area** → **Show Debug Area**).
 */
import RxSwift
/*:
 # Try Yourself
 
 It's time to play with Rx 🎉
 */
playgroundShouldContinueIndefinitely()

example("Try yourself") {
  let disposeBag = DisposeBag()
  Observable.just("Hello, RxSwift!")
    .debug("Observable")
    .subscribe()
    .disposed(by: disposeBag)
}


Review comment (bug):
Hi @devxoul ,

I think we can just write

```
// let disposeBag = DisposeBag()
_ = Observable.just("Hello, RxSwift!")
  .debug("Observable")
  .subscribe()
  // If dispose bag is used instead, sequence will terminate on scope exit
  // .disposed(by: disposeBag)
```

... or some comment worded better.

Code after:
/*:
 > # IMPORTANT: To use **Rx.playground**:
 1. Open **Rx.xcworkspace**.
 1. Build the **RxSwift-macOS** scheme (**Product** → **Build**).
 1. Open **Rx** playground in the **Project navigator**.
 1. Show the Debug Area (**View** → **Debug Area** → **Show Debug Area**).
 */
import RxSwift
/*:
 # Try Yourself
 
 It's time to play with Rx 🎉
 */
playgroundShouldContinueIndefinitely()

example("Try yourself") {
  // let disposeBag = DisposeBag()
  _ = Observable.just("Hello, RxSwift!")
    .debug("Observable")
    .subscribe()
    // .disposed(by: disposeBag) // If dispose bag is used instead, sequence will terminate on scope exit
}
Repository: ReactiveX/RxSwift
File: RxCocoa/iOS/UITextField+Rx.swift (Swift)

Code before:
    /// Reactive wrapper for `text` property.
    public var text: ControlProperty<String?> {
        return value
    }
    
    /// Reactive wrapper for `text` property.
    public var value: ControlProperty<String?> {
        return UIControl.rx.value(
            base,
            getter: { textField in
                textField.text
            }, setter: { textField, value in
                // This check is important because setting text value always clears control state
                // including marked text selection which is imporant for proper input 
                // when IME input method is used.
                if textField.text != value {
                    textField.text = value
                }
            }
        )
    }
    
    /// Bindable sink for `attributedText` property.
    public var attributedText: UIBindingObserver<Base, NSAttributedString?> {
        return UIBindingObserver(UIElement: self.base) { textField, attributedText in
            textField.attributedText = attributedText
        }
    }
    
}

#endif


Review comment (suggestion):
I think we need to find way to make sure this isn't updated in case attributed text is the same and test this.
```
if textField.attributedText != attributedText {
    textField.attributedText = attributedText
}
```

We had issues with IME.

```
// This check is important because setting text value always clears control state
// including marked text selection which is imporant for proper input 
// when IME input method is used.
```

Code after:
    /// Reactive wrapper for `text` property.
    public var text: ControlProperty<String?> {
        return value
    }
    
    /// Reactive wrapper for `text` property.
    public var value: ControlProperty<String?> {
        return UIControl.rx.value(
            base,
            getter: { textField in
                textField.text
            }, setter: { textField, value in
                // This check is important because setting text value always clears control state
                // including marked text selection which is imporant for proper input 
                // when IME input method is used.
                if textField.text != value {
                    textField.text = value
                }
            }
        )
    }
    
    /// Bindable sink for `attributedText` property.
    public var attributedText: ControlProperty<NSAttributedString?> {
        return UIControl.rx.value(
            base,
            getter: { textField in
                textField.attributedText
            }, setter: { textField, value in
                // This check is important because setting text value always clears control state
                // including marked text selection which is imporant for proper input
                // when IME input method is used.
                if textField.attributedText != value {
                    textField.attributedText = value
                }
            }
        )
    }
    
}

#endif
Repository: ReactiveX/RxSwift
File: Tests/RxCocoaTests/UITextView+RxTests.swift (Swift)

Code before:
    }

    func testDidChangeSelection() {
        var completed = false
        var value: ()?

        autoreleasepool {
            let textView = UITextView(frame: CGRect(x: 0, y: 0, width: 1, height: 1))

            _ = textView.rx.didChangeSelection.subscribe(onNext: { n in
                    value = n
                }, onCompleted: {
                    completed = true
                })

            textView.delegate!.textViewDidChangeSelection!(textView)
        }

        XCTAssertNotNil(value)
        XCTAssertTrue(completed)
    }
}

final class UITextViewSubclass2 : UITextView {
    var settedText = false
    var settedAttributedText = false
    
    override var text: String? {
        get {
            return super.text
        }
        set {
            settedText = true
            super.text = newValue
        }
    }
    
    override var attributedText: NSAttributedString? {
        get {
            return super.attributedText
        }
        set {
            settedAttributedText = true
            super.attributedText = newValue
        }
    }
}


Review comment (suggestion):
The same like for `UITextField`. This should probably be `didSetText` and `didSetAttributedText`.

Code after:
        var value: ()?

        autoreleasepool {
            let textView = UITextView(frame: CGRect(x: 0, y: 0, width: 1, height: 1))

            _ = textView.rx.didChangeSelection.subscribe(onNext: { n in
                    value = n
                }, onCompleted: {
                    completed = true
                })

            textView.delegate!.textViewDidChangeSelection!(textView)
        }

        XCTAssertNotNil(value)
        XCTAssertTrue(completed)
    }
}

private extension String {
    var enrichedWithTextFieldAttributes: NSAttributedString? {
        let tf = UITextView()
        tf.attributedText = NSAttributedString(string: self)
        return tf.attributedText!
    }
}

final class UITextViewSubclass2 : UITextView {
    var didSetText = false
    var didSetAttributedText = false
    
    override var text: String? {
        get {
            return super.text
        }
        set {
            didSetText = true
            super.text = newValue
        }
    }
    
    override var attributedText: NSAttributedString? {
        get {
            return super.attributedText
        }
        set {
            didSetAttributedText = true
            super.attributedText = newValue
        }
    }
}
Repository: ReactiveX/RxSwift
File: Tests/RxCocoaTests/UITextField+RxTests.swift (Swift)

Code before:
        ensurePropertyDeallocated({ UITextField() }, "a", comparer: { $0 == $1 }) { (view: UITextField) in view.rx.value }
    }
    
    func testSettingTextDoesntClearMarkedText() {
        let textField = UITextFieldSubclass(frame: CGRect.zero)
        textField.text = "Text1"
        textField.isSettedText = false
        textField.rx.text.on(.next("Text1"))
        XCTAssertTrue(!textField.isSettedText)
        textField.rx.text.on(.next("Text2"))
        XCTAssertTrue(textField.isSettedText)
    }
    
    func testLabel_attributedTextObserver() {
        let textField = UITextField()
        XCTAssertEqual(textField.attributedText, "".textFieldAttributedString)
        let attributedText = "Hello!".textFieldAttributedString
        _ = Observable.just(attributedText).bind(to: textField.rx.attributedText)
        XCTAssertEqual(textField.attributedText!, attributedText)
    }
}

private extension String {
    var textFieldAttributedString: NSAttributedString {
        let tf = UITextField()
        tf.attributedText = NSAttributedString(string: self)
        return tf.attributedText!
    }
}

final class UITextFieldSubclass : UITextField {
    var isSettedText = false
    
    override var text: String? {
        get {
            return super.text
        }
        set {
            isSettedText = true
            super.text = newValue
        }
    }
    
}


Review comment (bug):
I don't understand this code. `UITextField` is being created and then discarded immediately.

Is there something wrong with just using `NSAttributedText(string: "Hello!")`?

Code after:
    }
    
    func test_settingTextDoesntClearMarkedText() {
        let textField = UITextFieldSubclass(frame: CGRect.zero)
        textField.text = "Text1"
        textField.didSetText = false
        textField.rx.text.on(.next("Text1"))
        XCTAssertTrue(!textField.didSetText)
        textField.rx.text.on(.next("Text2"))
        XCTAssertTrue(textField.didSetText)
    }
    
    func test_attributedTextObserver() {
        let textField = UITextField()
        XCTAssertEqual(textField.attributedText, "".enrichedWithTextFieldAttributes)
        let attributedText = "Hello!".enrichedWithTextFieldAttributes
        textField.rx.attributedText.onNext(attributedText)
        XCTAssertEqual(textField.attributedText!, attributedText)
    }
}

private extension String {
    var enrichedWithTextFieldAttributes: NSAttributedString {
        let tf = UITextField()
        tf.attributedText = NSAttributedString(string: self)
        return tf.attributedText!
    }
}

final class UITextFieldSubclass : UITextField {
    var didSetText = false
    
    override var text: String? {
        get {
            return super.text
        }
        set {
            didSetText = true
            super.text = newValue
        }
    }
    
}
Repository: ReactiveX/RxSwift
File: Tests/RxCocoaTests/UIBarButtonItem+RxTests.swift (Swift)

Code before:
}

// UIBarButtonItem
extension UIBarButtonItemTests {
    func testBarButtonItem_DelegateEventCompletesOnDealloc() {
        ensureEventDeallocated({ UIBarButtonItem() }) { (view: UIBarButtonItem) in view.rx.tap }
    }
    
    func testButton_titleObserver() {
        let button = UIBarButtonItem()
        XCTAssertEqual(button.title, nil)
        let text = "title"
        _ = Observable.just(text).bind(to: button.rx.title)
        
        XCTAssertEqual(button.title, text)
    }
    
    func testBarButtonItem_actionExecution() {
        let button = UIBarButtonItem()
        weak var tapExpectation = expectation(description: "tap")
        let disposable = button.rx.tap.subscribe(onNext: {
            tapExpectation?.fulfill()
        })
        defer { disposable.dispose() }
        _ = button.target?.perform(button.action, with: nil)
        waitForExpectations(timeout: 1, handler: nil)
    }
}


Review comment (suggestion):
I think you can make it work without expectations e.g

```
let button = UIBarButtonItem()
var onNextCalled = false
let disposable = button.rx.tap.subscribe(onNext: {
    onNextCalled = true
})
_ = button.target?.perform(button.action, with: nil)
```

Code after:
}

// UIBarButtonItem
extension UIBarButtonItemTests {
    func testBarButtonItem_DelegateEventCompletesOnDealloc() {
        ensureEventDeallocated({ UIBarButtonItem() }) { (view: UIBarButtonItem) in view.rx.tap }
    }
    
    func testButton_titleObserver() {
        let button = UIBarButtonItem()
        XCTAssertEqual(button.title, nil)
        let text = "title"
        _ = Observable.just(text).bind(to: button.rx.title)
        
        XCTAssertEqual(button.title, text)
    }
    
    func testBarButtonItem_actionExecution() {
        let button = UIBarButtonItem()
        var onNextCalled = false
        let disposable = button.rx.tap.subscribe(onNext: {
            onNextCalled = true
        })
        defer { disposable.dispose() }
        _ = button.target?.perform(button.action, with: nil)
        XCTAssert(onNextCalled)
    }
}