[Kget] transfer status

[Felix Berger]

Hi,

two points of criticism as far as the transfer states are concerned:

You have write three lines of code to set a new state and notify listeners of 
it:

  tInfo.status = ST_Trying;
  setTransferChange(Tc_Status);
  emit transferChanged(this);

I'd suggest to provide just one (protected) change function that emits the 
signal. If necessary the signal could be suppressed depending on an optional 
boolean parameter at the end. I also don't see why the transfer change has to 
be so fine grained to report the kind of change.

void setTransferChange(int newState, const QString& stateString, bool 
emitSignal = true)
{
 tInfo.status = newState;
 if (emitSignal) {
  emit changed(this);
 }
}

Second point:

The list of states is limited and will always be, for example bittorrent has a 
state "hashing". That's why I would suggest only managing the states that are 
important from the outside (i.e. user interface), like active, stopped, or 
better resumable, stoppable,removable etc. So the userinterface knows what 
actions it can offer.

ST_Trying is not an important state for the userinterface and should be opaque 
from the outside. I'd rather have a status string I can set from the subclass 
like:

setTransferState(Stoppable|Active, i18n("Downloading..."));

or

setTransferState(Stoppable|Active, i18n("Hashing..."));

or

setTransferState(Resumable|Inactive, i18n("Paused"));

The last one is a direct reaction to a user action so it should maybe handled 
in the userinterface to guarantee the same state string everywhere though.

Another point: What's the advantage of providing the Info struct inside of 
Transfer, since it's not hidden in the cpp file, it doesn't help to keep 
binary compatibility for future versions and from a first glance it looks 
like a 1:1 relation between info and transfer. Am I missing something?

Felix

-- 
Try Debian GNU/Linux!
http://www.felix.beldesign.de/