Page MenuHomePhabricator

Descriptors cache invalidation & references
ClosedPublic

Authored by CLOVIS on Jul 3 2018, 3:29 PM.

Details

Summary

The code for the Descriptor & Reference classes.

Test Plan

Nothing to test.

Diff Detail

Repository
rLIBWFJVA WildFyre Java Libary
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Using static inner class

src/main/java/net/wildfyre/descriptors/Descriptor.java
15 ↗(On Diff #621)

You dont have to put the license in every file.

src/main/java/net/wildfyre/descriptors/Descriptor.java
24 ↗(On Diff #621)

Reason for this class having the public modifier?

src/main/java/net/wildfyre/descriptors/Descriptor.java
15 ↗(On Diff #621)

It's a standard in Java projects, I'm not sure why. Every Java IDE does that by default when you create the file. I can remove it if you prefer

24 ↗(On Diff #621)

Because it will be accessed from other packages:

net.wildfyre.api contains the main classes of the API (User, WFApi, ...)
net.wildfyre.descriptors contains the internal representation of the cache etc

So net.wildfyre.api needs to be able to access stuff in net.wildfyre.descriptors

src/main/java/net/wildfyre/descriptors/Descriptor.java
24 ↗(On Diff #621)

Somehow don't like having a class public that users shouldn't find

src/main/java/net/wildfyre/descriptors/Descriptor.java
24 ↗(On Diff #621)

There is no problem if they do, it won't break anything or enable access to secret information. I see what you mean though, I'll try to find a way to keep the architecture clean & hide internal classes.

src/main/java/net/wildfyre/descriptors/Descriptor.java
15 ↗(On Diff #621)

Kinda redundant tbh

WyldBot requested changes to this revision.Jul 7 2018, 11:26 PM
This revision now requires changes to proceed.Jul 7 2018, 11:26 PM
src/main/java/net/wildfyre/descriptors/Descriptor.java
15 ↗(On Diff #621)

StackOverflow says it's considered best practice to do it

Here too

CLOVIS added inline comments.
src/main/java/net/wildfyre/descriptors/Descriptor.java
24 ↗(On Diff #621)

Based on your concerns, I'm considering making this a subproject (alike the HTTP module). I have asked a question on SO about whether that's considered best practice, I'll get back to that task when I get some answers.

  • Also remove out/ dir of subprojects
  • Added subproject cache
  • Added subproject cache & dependencies
  • Merge branch 'master'
  • Better dependencies between projects
  • Moving package descriptors to new subproject cache/

@Info-Screen I moved the 'descriptors' package to a new subproject. The consequences is that even if its contents are public, they are only available from the current project (the lib) and not from the end-user's library (= if someone marks the lib as a dependency, the subprojects (http/ & cache/) are not imported)

This revision is now accepted and ready to land.Jul 11 2018, 10:11 PM
This revision was automatically updated to reflect the committed changes.