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

Question: Correct use of "Broken Chains" #44

Closed
richardtop opened this issue May 17, 2018 · 2 comments
Closed

Question: Correct use of "Broken Chains" #44

richardtop opened this issue May 17, 2018 · 2 comments

Comments

@richardtop
Copy link

Hi,
I've recently stumbled upon a specific use-case and decided to clarify on the correct use of the Promises. The use-case is the need to execute some code on the promise fulfillment and just propagate the error further if the promise has been rejected.

The example
I'd like to authenticate the user with the OAuth2 Password Grant, but keep the implementation details, such as the specific of the Token storage out of the ViewController.

So, let's consider, we have a LoginService:

class LoginService {
  lazy var auth = AuthService()
  lazy var client = RESTClient(accessToken: auth.token)

  func login(username: String = "", password: String = "") -> Promise<String> {
    weak var weakSelf = self
    return client.request(API.authenticate(username: username, password: password)).then({ (token) -> Promise<String> in
      let auth = weakSelf?.auth
      auth?.token = token
      return Promise<String>(token)
    })
  }

  @discardableResult func logOut() -> Promise<Void> {
    auth.eraseAllData()
    return client.request(API.logOut())
  }
}

The controller just uses login and logout methods of the service, while all the token handling is happening under the hood.

The question is, whether such use of Promises is correct, since I add specific logic only on success?

I've read about the Broken Chain antipattern, but would like to hear, whether the provided example is correct/incorrect way to handle such use-cases.

Thank you!
BR,
Richard

@shoumikhin
Copy link
Contributor

Hi Richard,

The code looks good. I'd only call out the issue if you possibly get another login invocation before the previous request had a chance to complete. May like to store the promise in an optional var and reject it on another login attempt. Something like the following:

class LoginService {
  // ...
  var loginPromise: Promise<String>?

  func login(username: String = "", password: String = "") -> Promise<String> {
    assert(Thread.isMainThread)
    loginPromise?.reject(...)
    auth?.token = ""
    weak var weakSelf = self
    let promise = self.client.request(
      API.authenticate(username: username, password: password)
    ).then { token in
      weakSelf?.auth?.token = token
      weakSelf?.loginPromise = nil
    }
    loginPromise = promise
    return promise
  }
  // ...
}

May also consider returning loginPromise if it's not nil and the username matches the previous invocation. So, everything depends on your use cases.

@richardtop
Copy link
Author

Thanks for clarifying the issue with another invocation. Practically, it's impossible, as the UI locks immediately before the login code is invoked. Other than this, I'm happy to hear that I use the library properly.

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

2 participants