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 0-suffixed fields to GetIdentity. #17

Merged
merged 3 commits into from
Aug 2, 2013

Conversation

Thynix
Copy link
Contributor

@Thynix Thynix commented Jun 27, 2013

The function is also used in GetIdentitiesByScore. Addresses bug #5729.

The function is also used in GetIdentitiesByScore. Addresses bug #5729.
@ghost ghost assigned xor-freenet Jul 4, 2013
@@ -326,36 +353,11 @@ private SimpleFieldSet handleGetIdentitiesByScore(final SimpleFieldSet params) t
if(getAll || score.getTrustee().hasContext(context)) {
// TODO: Allow the client to select what data he wants
final Identity identity = score.getTrustee();
sfs.putOverwrite("Identity" + i, identity.getID());
Copy link
Contributor

Choose a reason for hiding this comment

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

This line does not have an equivalent in your addIdentityFields() method!

Also moves adding the Identity field into the field-adding function.
@xor-freenet
Copy link
Contributor

Review results (will deal with fixing them, just want to write them down first):
The old code of getIdentitiesByScore did this:
sfs.putOverwrite("Nickname" + i, identity.getNickname() != null ? identity.getNickname() : "");

Your new code does:
sfs.putOverwrite("Nickname" + suffix, identity.getNickname());

It is questionable of whether this is the same.

@xor-freenet
Copy link
Contributor

More review results (will deal with fixing them, just want to write them down first):
getIdentitiesByScore iterates over a list of Score objects which it has received from the database and wants to send to the client.
Your new code does re-query each Score object from the database, which is very inefficient.

@xor-freenet
Copy link
Contributor

With regards to the nickname issue: The whole rest of the FCP interface does NOT do the empty string passing if the nickname is null. So it is a good idea to change this like you did. Also, Freetalk already has a proper handler for both cases, and it is the reference implementation of a FCP client. So we can keep the nickname part.

@xor-freenet
Copy link
Contributor

I've fixed the Score issue with commit e96e131

@xor-freenet
Copy link
Contributor

Also please notice the improvement of commit 09d752a

@xor-freenet xor-freenet merged commit 685056b into hyphanet:master Aug 2, 2013
@Thynix Thynix deleted the issue-5729 branch February 21, 2015 17:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants