Skip to content
This repository has been archived by the owner on Sep 14, 2022. It is now read-only.

Better documentation of available methods on 'swagger' object #145

Closed
myndzi opened this issue Jul 9, 2014 · 4 comments
Closed

Better documentation of available methods on 'swagger' object #145

myndzi opened this issue Jul 9, 2014 · 4 comments
Milestone

Comments

@myndzi
Copy link

myndzi commented Jul 9, 2014

In the readme docs, it is explained how to use swagger-node-express, but there seems to be no mention of the methods available on it. In fact, you wouldn't know there are any until you get to the examples and see a couple that are used without really any explanation.

Example: swagger.pathParam("petId", "ID of pet that needs to be fetched", "string")

This line is used but not explained. It is more or less obvious what it does, but the expected details are sorely lacking:

  • What exactly is the return value expected from this function? (This is necessary both in defining what the function does and in defining what the example expects!)
  • Are any of the parameters optional? Are there other parameters not used in this example?
  • Is there any validation? Can this method throw an error? (For example, if the path parameter you specify doesn't exist - it seems that a function referencing this information would reasonably validate it, but of course since it's being used IN the definition it doesn't have that information...)
  • Are there other methods that are not accidentally exposed by example code?

Then there's this one: swagger.errors.invalid('id')

  • It is used both as an object to throw and as an item to define the valid error responses. Does the module expect error objects as the parameters, or does the method return a plain object?
  • What happens if you want to throw an error with a stack trace? Does the error or object you throw have any meaning/relation to the object you specified in the schema? (It returns a new object every time, but the combination of definition and throw imply that it could return a reference to the same object used for strict comparisons, say.)
  • What other methods are there of swagger.errors, if any?

After browsing the code some, I found the answers to all of these questions, but it would be nice if they were documented up front, but there's a bigger question: do they serve a useful purpose at all?

In the code, the majority of these methods do nothing more than add the "type" value to the object returned -- but they require you to remember and correctly specify the order of the parameters, which aren't consistent across methods (See: placement of defaultValue). Frequently in node.js code at least, when you start to get a lot of parameters, you'll see an object literal being passed to help make explicit what each argument is:

swagger.formParam(name, description, type, required, allowableValuesEnum, defaultValue)
becomes:

swagger.formParam({
    name: 'name',
    description: 'description',
    type: 'type',
    required: !!required,
    allowableValuesEnum: [values],
    defaultValue: 'default'
})

But as you can see, at this point, an object literal is both more direct and cleaner. (P.S., the function I took this example from actually ignores the 'type' argument!)

Unless there is some special error handling that inspects the thrown objects, the module should keep out of throwing errors entirely or else provide some kind of structure for consistent error production. As it is, the example seems to imply this but none exists; nor is it clear that the error helpers accept a response object and proxy res.send -- another dubiously useful function since the differences between res.send(404, 'Thing not found') and swagger.errors.notFound('Thing', res) lean in favor of the former being both clearer about what is going on and more succinct.

In conclusion, I feel that the "helper" methods provided create more confusion than they solve and should probably be removed. If they remain, some stub of documentation to explain their purpose, parameter order, and return value would be nice. But all-in-all, it seems cleaner to me to be able to say "just do it like in the spec" instead of "just do it like in the spec but with some arbitrary functions thrown in that we haven't described"

@myndzi
Copy link
Author

myndzi commented Jul 9, 2014

P.P.S. The doc also references a now-broken link for the example about models: https://github.com/wordnik/swagger-node-express/blob/master/Apps/petstore/models.js

@paulhill
Copy link
Contributor

paulhill commented Jul 9, 2014

Best way to make it better is to submit a pull request.
The project could use your help :-)

@myndzi
Copy link
Author

myndzi commented Jul 9, 2014

I wouldn't mind, but this requires decision making on the part of whoever it is is deciding where this project goes...

@fehguy fehguy added this to the v2.1.4 milestone Jan 16, 2015
@fehguy
Copy link
Contributor

fehguy commented Jun 3, 2015

This is handled in #209 Now the spec is the source of truth for the server

@fehguy fehguy closed this as completed Jun 3, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants