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

Documenting keys in a return value #425

Closed
tkellen opened this issue Dec 8, 2011 · 8 comments
Closed

Documenting keys in a return value #425

tkellen opened this issue Dec 8, 2011 · 8 comments

Comments

@tkellen
Copy link

tkellen commented Dec 8, 2011

Say you have:
@return [Hash]

...and you want to document the keys within. I can't find in the docs anywhere that states how to do this. I've tried several different variations of @options whatever [type] key desc with no luck.

Wouldn't it make sense to have:
@options return [type] key description be supported in the same way it is for params?

@lsegal
Copy link
Owner

lsegal commented Dec 8, 2011

While I'm more or less on the fence with this proposal, I'm leaning heavily towards saying this should probably not be added, for a few reasons. First, a technicality: it would really be an "options hash", so @option wouldn't be the right thing to do here. It would be something of the form @return_hash. This might be a technicality, but it brings out the underlying problem with documenting opaque objects in the @return field. You're no longer documenting the method itself, but rather an interface returned by the implementation of an object. This is sort of the documentation equivalent of violating SRP, IMO. If we were to add this behaviour, it would still be confusing (again, IMO) for users to find this information, because they wouldn't be looking to find this in a return tag of method documentation.

The other issue with this is that it's extremely volatile documentation. What you are effectively doing by returning a Hash is defining an ad-hoc interface in your return field. It's not very reusable, and it's inherently uneasy to document-- and because documentation is communication, it's not very easy to communicate this to your users. I used a similar Hash returning method as an example of bad docs in my RubyConf 11 talk. This is a scenario where you will quickly lose sync with the implementation because of the volatile nature of a Hash interface, and it's even harder for your users to track what those changes are, if they occur.

If this is really necessary, you can define your own tags via the YARD commandline (or .yardopts). You could define a custom @return_hash that explains the contents of the hash. If you used a plugin, you could create a more featureful custom tag that worked like the options hash. I personally think this kind of behaviour belongs inside of a plugin, not the core features of YARD.

Another way to do this, and my personal recommendation, is to refactor any ad-hoc interfaces into real classes or modules. You don't strictly need to use these classes, they can be dummy classes, but then you can refer to those classes as the official "interface" for the object being returned. Consider:

# @return [MyInterface] the data returned from space!
def foo
  {:a => 1, :b => 2}
end

# @abstract
# @todo We could subclass OpenStruct here if we really wanted
class MyInterface
  # Documentation for the +a+ key
  attr_accessor :a

  # Documentation for the +b+ key
  attr_accessor :b

  # Hash-like interface to access keys in the +[]+ syntax.
  def [](key) self.send(key) end

  # Hash-like interface to access keys in the +[]=+ syntax.
  def []=(key, value) self.send("#{key}=", value) end
end

Note that we are still returning the object as a Hash (which might be a requirement if you're pulling this data from another lib or data source)-- but the interface is defined as being of type MyInterface (you might want to @note users of this fact). Of course the better way would be to just start returning the object using the class instead of just keeping it around as a dummy class-- then you have a much more well defined system!

The above are my objections-- but I'm open to your views too. Like I said, I'm on the fence about this, but I'm leaning pretty heavily to saying we should do this via plugin if at all.

@tkellen
Copy link
Author

tkellen commented Dec 10, 2011

While I agree that returning a hash may be an indication of poor OO practice, I don't agree that a documentation system should be enforcing coding standards.

It seems to me that providing the tag is unambiguously better than the arbitrary, inconsistent solutions out there like @returns [Hash<:Symbol Value, :Symbol Value>], @returns [Hash{:symbol=>value,:symbol=>value}], or even worse, just @returns [Hash] with nothing else.

That said, if something like this were added, I agree that making the tag return-value specific is more accurate than using @options.

Perhaps the more general name @return_detail could be used for appending an unordered list of items to a return value in the same way @options does for params?

@lsegal
Copy link
Owner

lsegal commented Dec 11, 2011

While I agree that returning a hash may be an indication of poor OO practice, I don't agree that a documentation system should be enforcing coding standards.

I suppose this is where we would disagree. It's certainly not enforcing any coding standards, because there are a hundred and one different ways to workaround this, but there is certainly a concept of "documentable" and "undocumentable" coding standards. Documentation is definitely tied and influenced by the way you structure your code (down to the way you name your code).

One of the goals of YARD is to improve the documentation in the Ruby community. In doing so, this also means attempting to break some of the bad coding practices in order to make code more documentable. This means getting rid of some poor documentation practices, but also means making recommendations about poor structural practices-- like abusing dynamic module inclusion (ie. ClassMethods & InstanceMethods) as well as the overuse of metaprogramming (which is inherently undocumentable). I would say that the approach YARD takes is often pragmatic, not idiomatic, and we try to bridge the gap as much as possible, but there are certainly practices that make for better documentation. I don't think YARD should encourage the breaking of these best practices, and for that reason, I think adding this kind of functionality into core would undercut this important goal of the project.

The way I see it, YARD takes on the Ruby principle of making ugly things look ugly, not by "enforcement", but just by "annoyance". You can document hashes in a return by either defining a custom tag or by marking up your own list in a @return tag, ie.:

# @return [Hash]
#   * :key [String] a key that accepts a string object
#   * :otherkey [Date] the date object to pass in
#   ...

It won't look quite as nice as options, but that's the whole "ugly things should look ugly" principle that Ruby follows, too.

That said, YARD also allows you to extend the behaviour and implement this kind of a feature through plugins and even custom tags on the command line. So you could (and quite easily, I might add) extend YARD to have an options tag named @return_detail that did this. I think this might be the best place for a feature that really isn't a best practice but still might be useful to some.

@lsegal
Copy link
Owner

lsegal commented Apr 28, 2013

Closing because this hasn't really gone anywhere in a while. Just to summarize, YARD isn't enforcing coding standards here, we just don't [currently] fully support weakly structured API designs. In other words, you can document the behaviour, it just won't look very pretty, because there is no special way to display these kinds of return values. Frankly, I'm not sure how to do this any better than we do right now, but if you can figure out a way, I would gladly look at a pull request for this.

@lsegal lsegal closed this as completed Apr 28, 2013
@ssimeonov
Copy link

+1 for a slightly different feature: specification of key and value types in hashes using the syntax @return Hash<KeyType => ValueType>. The generated documentation could use the very same syntax. It is simple, clear and readable. It also works recursively: Array< Hash <String => MyModel> >.

YARD already supports Array<ThisType, ThatType>. I don't see how this is different and have been bitten several times by documentation not being clear on the specifics of Hash returns. For example, a common use case when dealing with NoSQL backends and RESTful APIs wrapped by Ruby code is whether the keys are String or Symbol. Ruby likes Symbol but JSON parsing generates String. This mess won't end any time soon.

@lsegal the Hash<KeyType => ValueType> notation feels immediately natural. (I'm writing this because I tried it and found it did not work.) Do you see any problems with it? I'd be happy to start a new issue if that's a better place for the discussion.

@lsegal
Copy link
Owner

lsegal commented Oct 16, 2013

@ssimeonov YARD supports Hash{Key => Value} syntax in return tag type specifiers. See http://yardoc.org/types -- the reason => doesn't work with <> is because it's ambiguous. Generally type specifiers are freeform text, except for weird parsing edge cases like those.

@lsegal
Copy link
Owner

lsegal commented Oct 16, 2013

Note that you can also just use the more "generic friendly" style of Hash<Key, Value>

@touredev
Copy link

#   * :key [String] a key that accepts a string object
#   * :otherkey [Date] the date object to pass in

ail

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants