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

HystrixObservableCommand.run() Method Name #223

Closed
benjchristensen opened this issue Mar 14, 2014 · 9 comments
Closed

HystrixObservableCommand.run() Method Name #223

benjchristensen opened this issue Mar 14, 2014 · 9 comments

Comments

@benjchristensen
Copy link
Contributor

Should this method still be called run(), or something else since it actually does not "run" anything, but just retrieves the Observable<T> that will later be run?

@benjchristensen
Copy link
Contributor Author

This needs to be decided before the final 1.4.0 release.

@alexwen
Copy link

alexwen commented Mar 14, 2014

The pattern i've been using for commands which return an Observable (and don't actually run) is to call them, prepareX(). X being the action they are preparing, so in this case, it would be prepareRun()

There may be a better convention for reactive commands though that I am not aware of. :) But I agree it should be renamed to reduce confusion.

@benjchristensen benjchristensen added this to the 1.4 milestone Sep 1, 2014
@spencergibb
Copy link
Contributor

get() or getObservable()?

@benjchristensen
Copy link
Contributor Author

Either of those would work for me. So it seems options are:

  • get()
  • getObservable()
  • prepare()
  • prepareRun()
  • prepareObservable()
  • create()

@KoltonAndrus
Copy link
Contributor

observe() or create() would be my vote. The class name implies that you're creating an observable, so I don't think you need to include 'observable' in the method name. Unless you think it removes ambiguity when compared to the other Hystrix Commands.

@mattrjacobs
Copy link
Contributor

What about construct()? Like a constructor, you pass it arguments, but don't expect anything to run until you call other methods.

@benjchristensen
Copy link
Contributor Author

construct() sounds good too.

@pmotyka
Copy link

pmotyka commented Sep 24, 2014

+1 for construct(), I think this best expresses the behavior of the method.

@benjchristensen
Copy link
Contributor Author

I like construct() as well.

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

6 participants