Processing HI1 and then HI

Problems compiling? Don't understand the source code? Don't know how to code your feature? Post here.

Moderator: Moderators

Locked
ullner
Forum Moderator
Posts: 333
Joined: 2004-09-10 11:00
Contact:

Processing HI1 and then HI

Post by ullner » 2006-04-09 12:36

Currently, DC++ will not add a user to the userlist if they send HI1 and then HI in ADC hubs. When I had completed the fix, I looked at it and felt I violated the DRY principle. So, I changed the code to (try to) not violate the DRY principle, but I believe it added complexity.

Everything happens in HubFrame.cpp.

The first code;

Code: Select all

if(!ui->getIdentity().isHidden() && u.identity.isHidden() && showUsers) {
	ctrlUsers.deleteItem(ui);
}
		
if(ui->getIdentity().isHidden() && !u.identity.isHidden() && showUsers) {
	ctrlUsers.deleteItem(ui);
	ctrlUsers.insertItem(ui, getImage(u.identity));
}
I added the second if-block. It's quite easy to see what the code does, but like I said, it feels like it violates the DRY principle.

The second code;

Code: Select all

if((ui->getIdentity().isHidden()^u.identity.isHidden()) && showUsers) {	// XOR-ing... If they are different, proceed
	ctrlUsers.deleteItem(ui);
	if(ui->getIdentity().isHidden() && !u.identity.isHidden()) {	// Has the user been hidden and is not now?
		ctrlUsers.insertItem(ui, getImage(u.identity));
	}
}
While this code is more straight forward, it's difficult to understand what it does on first glance.

What do you guys think? (I was hoping one of them would make it into DC++, considering the issue with HI1 and HI. Of course, I would make a proper diff and Bugzilla entry.)

Carraya
Posts: 112
Joined: 2004-09-21 11:43

Post by Carraya » 2006-04-09 13:41

I see the dilema, for some reason I like the first one better, probably because you can see what it does right away, where the second one looks more complex...
<random funny comment>

joakim_tosteberg
Forum Moderator
Posts: 587
Joined: 2003-05-07 02:38
Location: Sweden, Linkoping

Post by joakim_tosteberg » 2006-04-09 14:50

I agree with Carraya that the first code feels better due to the fact that it is easier to understand. But exchanging the xor in the second code for != might make it slightly more readable.

ullner
Forum Moderator
Posts: 333
Joined: 2004-09-10 11:00
Contact:

Post by ullner » 2006-04-09 15:16

I considered doing that too, and in retrospect, I don't know why I didn't write that in my original post.

Yes, the first code feels easier, but I find it annoying having so much repetitiveness.

FarCry
Programmer
Posts: 34
Joined: 2003-05-01 10:49

Post by FarCry » 2006-04-10 06:05

I don't want the client to hide any information from me just because the hub or another client asks it to. Therefor I'd like to have users with HI1 displayed with a different icon and sorted to the bottom of the list by default.

Or alternatively ignore HI1 in dc++ completely.

Or remove HI from the protocol.

ullner
Forum Moderator
Posts: 333
Joined: 2004-09-10 11:00
Contact:

Post by ullner » 2006-04-10 06:18

I'd prefer the third choice if I had to choose between them.

Your first suggestion implies that the hub should also be shown in the userlist as a "user"...

Locked