Skip to content

Cleanup, track dirty objects - #3

Open
jbedard wants to merge 4 commits into
joshcbarnes:replaceJdofrom
jbedard:replaceJdo
Open

jbedard wants to merge 4 commits into
joshcbarnes:replaceJdofrom
jbedard:replaceJdo

Conversation

@jbedard

@jbedard jbedard commented Jul 9, 2016

Copy link
Copy Markdown
Contributor

return getInstanceCache(clazz, id).computeIfAbsent(Objects.requireNonNull(id), fetcher.andThen(Objects::requireNonNull));
}

public <T extends PersistedObject> void addDirty(T po) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was debating adding the po.setDirty(field) to this call as well (and adding a String field param). That way the aspect wouldn't have any logic in it and only provides a (nice compile time) method of setting the dirty state.

But is this object public? Is it ok having this on the public class?

WDTY?

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're talking about po.setDirty? I think it's probably fine having that public since it only gets added to the class as part of the weaving process...i.e. I don't think eclipse will show that method as being available

or am I misunderstanding?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess I was combining two separate things:

  1. Somehow we should expose getObjectById like JDO does, probably along with save and delete. Which object will expose that? And this addDirty should not be exposed despite the implementation currently being beside getObjectById
  2. I think the logic in PersistanceAspect#markFieldDirty should move here so that a) the aspect has no logic in it b) both the dirty field and dirty object logic is in one spot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants