swagger-api / swagger-parser

Swagger Spec to Java POJOs
http://swagger.io
Apache License 2.0
778 stars 525 forks source link

Very low code quality #210

Closed bademux closed 8 years ago

bademux commented 8 years ago

Trying to use it in my project (firstly attempt to use swagger-codegen, but it just a mess).

  1. Swagger parser ignores KISS. Is there any good reason to handle URL? It must grab InputStream (String, byte[]). Downloading and Authenticating are external external (to swagger parser) responsibilities.
  2. "wtf".equals(wtf.wtf().wtf()) source
"2.0".equals(output.getSwagger().getSwagger())
  1. Luke use Exceptions
        output = new SwaggerDeserializationResult();
        output.message("The swagger definition could not be read");
        return output;
  1. Disabled test?! @fehguy Why you do it at "3 Nov 2015" and when you plan to enable it? There is no good reason to "disable" test, but if you do, then use @Ignore
fehguy commented 8 years ago

You can contribute back or find an alternative. I'm not making you use the code, and that attitude isn't very OSS friendly

bademux commented 8 years ago

Looks like a lot of things are unfixable without breaking compatibility. It is better to cooperate for fixing where it possible (or plan breaking fixes for further release) , then take offense by closing the issue.

fehguy commented 8 years ago

It's really simple. If you have constructive input, an issue is good. If you have contributions, a pull request is good. If you propose breaking compatibility then a conversation is good. But if you want to complain just to complain, use your diary instead of this github project

bademux commented 8 years ago

Please. ask to the first 4 points. I'm simply don't understand 1.why parser library downloads json\yaml

  1. is there possibility to break APIs (yeah .getSwagger().getSwagger()) isn`t unique )
  2. why SwaggerDeserializationResult is used
  3. commit says "disable test", cool but, again, why?
fehguy commented 8 years ago

For item, 1:

https://github.com/swagger-api/swagger-parser#overview

This project's goal is to download definitions, validate and return POJOs. To do that, because of relative and remote references, it needs to read references, which may be JSON, YAML, and authenticated.

For item 2:

Yes, but like all libraries, you should deprecate existing, old APIs and it requires a version bump.

For item 3:

I don't know what the question is

For item 4:

There is an issue with Mokito implementation in that test.

bademux commented 8 years ago
  1. Fix me if I'm wrong. There are no basic parser without doing all the staff around. (If you will ask me, I suggesting resolve external references can be by handlers)
  2. It is long story... Maybe someone already attempts to write solid parser for OpenAPI in java?
  3. The question is about strange returning SwaggerDeserializationResult .
  4. How about removing it from the repository then? ...

The whole code base as well as project itself (java part) lacks of review and. Suggesting to run findbug or just IntelliJ Idea Analyzer (~30 000 warnings and errors), from stupid like public static enum to serious like if (str == "").