Unused code in shared and protocol modules #1374

New Issue

2018-10-12T16:04:23+02:00

g1itch commented

2018-10-12 16:04:23 +02:00

(Migrated from github.com)

Hello.

The functions checkAndShareObjectWithPeers, _checkAndShareUndefinedObjectWithPeers, _checkAndShareMsgWithPeers, _checkAndShareGetpubkeyWithPeers, _checkAndSharePubkeyWithPeers, _checkAndShareBroadcastWithPeers are same in protocol and shared modules and used from latter as I can see.

Remove it from protocol?

Hello. The functions `checkAndShareObjectWithPeers`, `_checkAndShareUndefinedObjectWithPeers`, `_checkAndShareMsgWithPeers`, `_checkAndShareGetpubkeyWithPeers`, `_checkAndSharePubkeyWithPeers`, `_checkAndShareBroadcastWithPeers` are same in `protocol` and `shared` modules and used from latter as I can see. Remove it from `protocol`?

g1itch commented

2018-10-12 16:47:19 +02:00

(Migrated from github.com)

The only place where checkAndShareObjectWithPeers() appears is class_objectProcessor.py#L751 but broadcastToSendDataQueues seems to be useless, because I cannot find where sendDataQueues are handled.

The only place where `checkAndShareObjectWithPeers()` appears is [class_objectProcessor.py#L751](../blob/v0.6/src/class_objectProcessor.py#L751) but `broadcastToSendDataQueues` seems to be useless, because I cannot find where `sendDataQueues` are handled.

PeterSurda commented

2018-10-12 17:26:52 +02:00

(Migrated from github.com)

In general shared should be reduced as much as possible. Just check that the code in protocol is working correctly and nothing references the removed functions from shared.

Regarding check*, this is supposed to be obsoleted because there are new check implementations in BMObject. As far as I can see, there is only one reference to them, from class_objectProcesor when you extract an ACK from a message, you call these checks before putting the ACK into the inventory. This should be refactored to use BMObject (maybe some extra coding is necessary, I'm not sure if they are compatible, I think I didn't pay enough attention during moving to asyncore), and the checks from both protocol and shared should be removed.

Regarding broadcastToSendDataQueues and sendDataQueues, I'm pretty sure they are obsolete as asyncore doesn't use them anymore. Maybe they are even creating a memory leak.

In general `shared` should be reduced as much as possible. Just check that the code in `protocol` is working correctly and nothing references the removed functions from shared. Regarding `check*`, this is supposed to be obsoleted because there are new check implementations in `BMObject`. As far as I can see, there is only one reference to them, from `class_objectProcesor` when you extract an ACK from a message, you call these checks before putting the ACK into the inventory. This should be refactored to use `BMObject` (maybe some extra coding is necessary, I'm not sure if they are compatible, I think I didn't pay enough attention during moving to asyncore), and the checks from both `protocol` and `shared` should be removed. Regarding `broadcastToSendDataQueues` and `sendDataQueues`, I'm pretty sure they are obsolete as asyncore doesn't use them anymore. Maybe they are even creating a memory leak.

PeterSurda commented

2018-10-12 17:33:25 +02:00

(Migrated from github.com)

I looked at it and it definitely does need extra work as the BMObject constructor requires decoded arguments, and the decoder only works on buffers. So some redesign is necessary if we're to avoid duplication of code.

I looked at it and it definitely does need extra work as the `BMObject` constructor requires decoded arguments, and the decoder only works on buffers. So some redesign is necessary if we're to avoid duplication of code.

PeterSurda commented

2018-10-12 17:37:47 +02:00

(Migrated from github.com)

Maybe create a fake BMProto object o, copy the buffer from the ACK buffer (or just reference it), and call o.bm_command_object() to process it. Probably some additional tests are necessary.

Maybe create a fake `BMProto` object `o`, copy the buffer from the ACK buffer (or just reference it), and call `o.bm_command_object()` to process it. Probably some additional tests are necessary.

g1itch commented

2018-10-12 17:41:38 +02:00

(Migrated from github.com)

I found that I do not understand where objects are broadcasted to the network ):

g1itch commented

2018-10-18 09:06:13 +02:00

(Migrated from github.com)

I have a rather controversial solution: 03ee14e9.

This repo is archived. You cannot comment on issues.