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

Add methods to list the keys #28

Merged
merged 3 commits into from
Dec 27, 2016
Merged

Add methods to list the keys #28

merged 3 commits into from
Dec 27, 2016

Conversation

juanet3
Copy link
Contributor

@juanet3 juanet3 commented Dec 15, 2016

Solving some midnight bugs

@coveralls
Copy link

coveralls commented Dec 15, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling a9c9cc5 on juanet3:master into 90814f1 on Fewlaps:master.

@juanet3
Copy link
Contributor Author

juanet3 commented Dec 15, 2016

This solves malformed code and it runs!

@juanet3 juanet3 closed this Dec 15, 2016
@juanet3 juanet3 reopened this Dec 15, 2016
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling a9c9cc5 on juanet3:master into 90814f1 on Fewlaps:master.

1 similar comment
@coveralls
Copy link

coveralls commented Dec 15, 2016

Coverage Status

Coverage remained the same at 100.0% when pulling a9c9cc5 on juanet3:master into 90814f1 on Fewlaps:master.

@@ -80,6 +83,33 @@ public void set(String key, T value, long keepAliveInMillis) {
}
}

public List<String> listCachedKeysStartingWith(String keyStartingWith) {
List<String> keys = new ArrayList<String>();
keyStartingWith = getEffectiveKey(keyStartingWith);
Copy link
Member

Choose a reason for hiding this comment

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

Why not calling it "key" or "effectiveKey" ? Reusing an input parameter is not an issue in this case, but if we call it "effectiveKey" we will be sure that the key has been translated when using it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Double morality Mr. Roc?

public void set(String key, T value, long keepAliveInMillis) {
key = getEffectiveKey(key);

from QNCache.java

Copy link
Member

Choose a reason for hiding this comment

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

Hahahaha nope! Evolution! Btw, thanks for spotting it! :-)


for (String key : Collections.list(cache.keys())) {
if (key.startsWith(keyStartingWith)) {
keys.add(key);
Copy link
Member

Choose a reason for hiding this comment

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

Wep, autoformat the code, please :·D


public List<String> listCachedKeysStartingWithIfAlive(String keyStartingWith) {
List<String> keys = new ArrayList<String>();
final long now = now();
Copy link
Member

Choose a reason for hiding this comment

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

Why final? We could avoid this as it doesn't add any value... just code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Old java 8 code needs it and I forgot to change.

return keys;
}

public List<String> listCachedKeysStartingWithIfAlive(String keyStartingWith) {
Copy link
Member

@rocboronat rocboronat Dec 16, 2016

Choose a reason for hiding this comment

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

What do you think about listAliveCachedKeysStartingWith ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Waw!

It will clarify its behavior. ListCachedKeys and ListCachedKeysStartingWith?

Roc from past says!

Copy link
Member

Choose a reason for hiding this comment

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

Hihi and now, looking at the implementation of Java's Map, I see that Java call this method keySet(). What do you think about calling it keySetStartingWith? Some thoughts there...

@rocboronat
Copy link
Member

@juanet3 thanks for the code!

BTW, what do you think about removing the "cached" word of the methods? When the people uses this librarby, it would write something like cache.listCachedKeys(). It's great, but cache.listKeys() is greater :·D. Just an opinion :·D

BTW, could you add a simpler method listKeys() and listAliveKeys()? If not, no problem, I could do it too hihi

@rocboronat
Copy link
Member

@juanet3 as your PR gives a lot of value to the project, do you want me to implement my own suggetions? It would be great to launch a release with your additions :·)

@rocboronat rocboronat changed the title Master Add methods to list the keys Dec 18, 2016
@rocboronat
Copy link
Member

Going to merge these changes and apply my suggestions to master :·)

@rocboronat rocboronat merged commit 6ce66fb into fewlaps:master Dec 27, 2016
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.

3 participants