Modularize JHotDraw with Java Platform Module System (JPMS) - #35
Modularize JHotDraw with Java Platform Module System (JPMS)#35taherehbi1994 wants to merge 1 commit into
Conversation
wumpz
left a comment
wumpz
left a comment
There was a problem hiding this comment.
Looking at the branch network JHotdraw seems to be a project in multiple student tasks. Nice.
Since we use it in a non JPMS project, this was never a problem or an issue.
What kind of task do you want to achieve JPMS modularizing JHotdraw?
Looking forward hearing from you.
Tobias
| if (c != null && c.isEnabled()) { | ||
| Transferable t = ClipboardUtil.getClipboard().getContents(c); | ||
| if (t != null && c.getTransferHandler() != null) { | ||
| if (c.getTransferHandler().importData(c, t)) importDataSuccess(); |
There was a problem hiding this comment.
Why did you revert my recent changes?
There was a problem hiding this comment.
Hi Tobias,
Just to clarify the overall approach: for the modularization itself, we don't need to touch the logic of the code or change anything inside the methods.
The only changes involved are:
- Adding module-info.java files to declare dependencies and exports
- Adjusting pom.xml for module support
- In some cases, renaming a test's package when it collides with the main class's package, to avoid a split-package error under JPMS
So any place where a line looks removed or changed in the diff (like the importData call in PasteAction.java) wasn't an intentional edit on my part - it's most likely because my branch was based on an older point in develop and doesn't include some of your more recent commits. I'll rebase on the latest develop so the diff only reflects the actual modularization changes.
| @@ -17,7 +17,7 @@ | |||
| * MA 02110-1301 USA | |||
| */ | |||
|
|
|||
| package org.jhotdraw.io; | |||
There was a problem hiding this comment.
Why do you move tests in a subpackage test? Since tests should be in the same package as the tested class. This has also something to do with visibility and maven standards.
There was a problem hiding this comment.
This is because of a JPMS rule: two modules can't share the same package name (a "split package", which the compiler rejects). Since the tests are being modularized into their own module, and org.jhotdraw.io is already owned by the main module, the test module needs a different package name, hence org.jhotdraw.io.test.
There was a problem hiding this comment.
Sorry for keeping you waiting. ;)
I do not like the tests in a different package. This goes against maven standards. Developers, IDEs are expecting the directory structure of a familiar kind.
As far as I know, the surefire plugin somehow injects the tests into the main module, when the tests do not have a module-info.java file. Its called patching. So the test classes could be in the same package as the tested classes. That would be suitable and not break the standards. Right?
Building multiple larger Server/Client applications, the use of JPMS was til now never needed or even could not be acomplished in a meaninful manner and time. So what is the advantage using JPMS while most of the libraries you are using for an application are not using it? Using an automatic module name introduces other problems. For instance in some configurations you get a version at the module name generated from the base package.
Sure, JPMS gives you more control over which parts of your module you open to the outside. However, to use JPMS you need to tackle those problems mentioned above. At the moment the only real advantage of JPMS was indeed the modularization of the JDK. IMHO
Tobias
2 participants
Hey there 👋
I've been working on modularizing JHotDraw using JPMS (Java Platform Module System) as part of my PhD research. Each sub-module now has its own module-info.java, so dependencies and exported packages are explicit.
It was a fun (and sometimes tricky!) exercise figuring out the right module boundaries for this codebase. Would love to hear your thoughts on it, and I'm happy to adjust anything that doesn't fit well with the project's direction.
Thanks for maintaining such a great project!