-
Notifications
You must be signed in to change notification settings - Fork 0
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
Ignore no-op load/save/delete actions properly #1
base: master
Are you sure you want to change the base?
Conversation
@fpeter8 pls, thx |
@@ -122,6 +123,10 @@ | |||
for (S id: ids) | |||
keymap.put(this.<T>makeKey(id), id); | |||
|
|||
if (keymap.isEmpty()) { | |||
return Maps.newLinkedHashMap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Collections.emptyMap();
? Or you want it to be writable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TBH there is no proper consistency in the objectify itself, for example
some uses empty map, some new hashmap, but this method returned this
originally, so seemed like a good idea to keep it.
2015.10.19. 13:18 ezt írta ("Peter Farkas" [email protected]):
In src/main/java/com/googlecode/objectify/impl/LoadTypeImpl.java
#1 (comment):@@ -122,6 +123,10 @@
for (S id: ids)
keymap.put(this.makeKey(id), id);
if (keymap.isEmpty()) {
return Maps.newLinkedHashMap();
Collections.emptyMap();? Or you want it to be writable?
—
Reply to this email directly or view it on GitHub
https://github.com/nbali/objectify/pull/1/files#r42359680.
please follow code conventions (one-line ifs don't have brackets) |
Funny part again, sometimes they dont, sometimes they do ;-)
|
why did you modify LoadTypeImpl? It calls LoaderImpl (which contains the empty clause already). If this is such an improvement, why not SaveTypeImpl and DeleteTypeImpl? |
I wanted to avoid the unnecessary proxying, but you are right test should
|
c4d5d90
to
4c65d23
Compare
@fpeter8 next round pls |
* the goal is to avoid the small, but potentially frequently executed calls to save CPU * removal of proxy creation in favor of forwarding list/map
I'll need to check the Round implementation to be sure now - nowUncached change is fine, the rest seems OK |
@nbali FYI I've updated the master branch |
This change is