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

Generate inner public static classes for object properties. #125

Open
dant3 opened this issue Sep 18, 2013 · 20 comments
Open

Generate inner public static classes for object properties. #125

dant3 opened this issue Sep 18, 2013 · 20 comments

Comments

@dant3
Copy link

dant3 commented Sep 18, 2013

If property of type object does not have any ref or common id, I think is't better to create inner public static class Instead of package level classes. This way multiple schemas won't clash in class names if you have to create classes for object based on property name (like it works nowadays).

Example:

one.json

{
    "type" : "object",
    "properties" : {
        "data" : {
            "type": "object",
            "properties" : {
                "something" : {
                    "type" : "number"
                }
            }
        }
    }
}

two.json

{
    "type" : "object",
    "properties" : {
        "data" : {
            "type": "object",
            "properties" : {
                "other" : {
                    "type" : "string"
                }
            }
        }
    }
}

That I'm getting is two classes: Data and Data_ in a package scope. This does not makes much sence, and is impossible to use. You can use javaType workaround, but it's not so good either - there is not much type names you can think of.
IMHO the best solution for this problem is to generate inner public static classes inside class for parent schema, getting One.Data class and Two.Data class. This way it is readable/understandable and does not clashes.

Possibly duplicate of #22 as well.

@joelittlejohn
Copy link
Owner

I definitely like the idea of this. It's obviously a significant, breaking change to the way the classes are currently generated so we should probably add it as a config option. I think this would be a very useful option though.

@jkrizanic
Copy link

@joelittlejohn Is this config option which you mentioned added?

@joelittlejohn
Copy link
Owner

Nope, no work has been done on this feature.

@jkrizanic
Copy link

Is it going to be in the next release?

@joelittlejohn
Copy link
Owner

Unless someone provides an implementation, I think it's safe to say, no.

@zhouzhipeng
Copy link

I've implemented the "inner class " feature in my fork. here is the pull request: #1008

@tviegut
Copy link

tviegut commented Aug 8, 2019

@zhouzhipeng Thanks for your efforts toward providing "inner class" support.

@joelittlejohn Any thoughts on if/when you may accept the pull request?

@joelittlejohn
Copy link
Owner

@tviegut Thanks for the nudge. I've had a quick look through that PR and added some comments on what's left to do.

@tviegut
Copy link

tviegut commented Aug 16, 2019

@joelittlejohn No problem and thanks for following up.

@saberya
Copy link

saberya commented Nov 2, 2021

@joelittlejohn is there a plan to merge this feature in the next release? nested classes is important when i generate a single java file from front request body.

@wigbam
Copy link
Contributor

wigbam commented Feb 25, 2022

Is there any update on this? Any plans to get the PR merged?

@wigbam
Copy link
Contributor

wigbam commented Feb 27, 2022

I have taken a shot at it:
#1380

Please can somebody have a look?

@hupfdule
Copy link

@joelittlejohn There are now two PRs for this issue (#1160 and #1380) apart from #1008 (that didn't fulfill all your requirements).

It would be nice to review them, comment on which one you would integrate and what changes you still request to integrate them. There is obviously some interest in that feature, so I would to see it integrated in one of the next releases.

@UnasZole
Copy link

Hi @joelittlejohn ,
Can you please review the PR #1380 that is pending to add this feature ?
It's been sitting here for more than a year, and it's definitely a very important feature.

If there are some changes left to do, and in case @wigbam is no longer available to work on it, I may contribute as well, but I'd like to be sure that you'll at least look at it before I create yet another PR on the subject ;-)

@wigbam
Copy link
Contributor

wigbam commented Mar 15, 2023

Funny you mentioned it! I did kind of forget about it for some time, but just yesterday I re-stumbled upon and was planning on getting the PR back into shape this week. I will address the comments and rebase.

@wigbam
Copy link
Contributor

wigbam commented Mar 16, 2023

The PR has been rebased and cleaned up. It is ready for another 👀

@joelittlejohn

@UnasZole
Copy link

UnasZole commented May 4, 2023

Hi @joelittlejohn ,

Can you please at least acknowledge @wigbam 's pull request #1380 and give some kind of status ? What's preventing you from reviewing and/or merging the 1-year-old fix to a 10-year-old issue, which already underwent a significant code review ?

Don't get me wrong, I perfectly understand lacking time for doing a rather big code review yourself, but it's quite rude to completely ignore contribution proposals like this while you're still active on the project and reacting on other issues...

@joelittlejohn
Copy link
Owner

@UnasZole This change has never been a priority for me since (as I've mentioned on many issues in the past), we can't support every possible style of Java people prefer. It adds a maintenance burden and I have no doubt there are more edge cases to be discovered here. It's great that Stepan has shared his code for you to use.

Please note though, the thousands of hours I have already contributed to building this tool do not entitle you to more of my time.

@UnasZole
Copy link

UnasZole commented May 5, 2023

Thanks for your answer.

As I said, I fully understand not committing all your time to an open source project - that's completely normal. And I didn't blame you for the waiting time in itself; I complained specifically about the lack of answer, that leaves all other potential contributors and users in limbo, unsure of where they should invest their time, and losing their own time in the process.

In that regards, a clear "I don't have time right now" or "I don't want to pay the maintenance cost" answer is perfectly fine, and helps me and anyone else to focus on appropriate solutions.
So thanks again, that answer is all that I needed.

@Igor-Mukhin
Copy link

Guys, die you find any workaround? What do you do to avoid those conflicts and __1, __2 classes?

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

10 participants