The Destructive Getter



  • While slogging through the amazingly old and complex source code of the Azureus (now: Vuze) open source java BitTorrent client, I found this little getter method:

    private ArrayList chokes = new ArrayList();
    [..snip..]
    public ArrayList getChokes() {
        ArrayList to_choke = chokes;
        chokes = new ArrayList();
        return to_choke;
    }
    

    source



  • Maybe I'm missing something, but I don't really see anything weird here, unless you don't want it to clear out the list when you get it.  Without seeing the rest of the code, I don't know if that is the case or not.



  • I think the entire point of having a getter is about getting something, as compared to (re)setting stuff with a setter. Of course, this wouldn't be as irritating if the method wasn't named to look just like any innocent getXYZ method...



  •  While I agree getXXX shouldn't be the name of such a method I don't think it's really a problem since it looks like expected behaviour when you look at the rest of the code.



  • Looks like someone wanted to implement a spool.  I'm betting there's a corresponding addChoke() method there too: you call addChoke() to queue a choke (whatever that is) for later processing, and at some convenient point some other piece of code calls getChokes() to get all the chokes that have been queued since the last call and handles them in one batch.  Yeah, the name is kind of misleading, but it's hard to say exactly what name would've been better.

    ...OK, just looked at the code.  I guess I was wrong, and I really have no idea WTF's actually going on in there.  I kind of suspect some piece of code was processing chokes twice, and rather than fix the actual bug, they just kluged it so that the second call to getChokes() would always return an empty list.



  • @pedric said:

    While slogging through the amazingly old and complex source code of the Azureus (now: Vuze) open source java BitTorrent client, I found this little getter method:

    private ArrayList chokes = new ArrayList();
    [..snip..]
    public ArrayList getChokes() {
    ArrayList to_choke = chokes;
    chokes = new ArrayList();
    return to_choke;
    }

    source

    That's just like one of those hardware registers where the flag bits auto-reset to zero any time you read it.  I hate those. 



  •  We can't tell whether this is a WTF or not without knowing what a 'choke' is. If a choke is a thing that can obviously only be in one place at a time, then the implementation is precisely correct. If a choke is a thing that can only be in one place at a time for non-obvious and subtle reasons, then perhaps the name 'getChokes' is poorly chosen, but otherwis, there's no WTF. If there's no logical reason you wouldn't want to or be able to non-destructively get chokes, then at least the name is a significant WTF.

     People would expect a 'getX' function to be nondestructive unless 'X' were such that a non-destructive get is obviously impossible or nonsensical.

     So WTF ... is a choke?

     Update: From my quick scan, I gather that a 'choke' is a BT protocol message that cuts off a leecher. I suspect this is code that accumulates the list of pees to choke (rather than the list of peers that are choked) and the only time the list is retrieved is to send it. So the WTF is the method name. Though it's hard to think of a better method name that's not awkward. takeChokes? flushChokes?



  • @joelkatz said:

     So WTF ... is a choke?

    Vot is a "choke"?  You don't know vot is a "choke"?  Oi, vat a schmuck!  It's ven ve make a funny, you laugh, ha-ha, no?

     



  • @joelkatz said:

     We can't tell whether this is a WTF or not without knowing what a 'choke' is. If a choke is a thing that can obviously only be in one place at a time, then the implementation is precisely correct. If a choke is a thing that can only be in one place at a time for non-obvious and subtle reasons, then perhaps the name 'getChokes' is poorly chosen, but otherwis, there's no WTF. If there's no logical reason you wouldn't want to or be able to non-destructively get chokes, then at least the name is a significant WTF.

     People would expect a 'getX' function to be nondestructive unless 'X' were such that a non-destructive get is obviously impossible or nonsensical.

     So WTF ... is a choke?

     Update: From my quick scan, I gather that a 'choke' is a BT protocol message that cuts off a leecher. I suspect this is code that accumulates the list of pees to choke (rather than the list of peers that are choked) and the only time the list is retrieved is to send it. So the WTF is the method name. Though it's hard to think of a better method name that's not awkward. takeChokes? flushChokes?

     

     



  • @joelkatz said:

    So the WTF is the method name. Though it's hard to think of a better method name that's not awkward. takeChokes? flushChokes?

    removeChokes()? popChokes()? It's working as a Queue or a Stack, basically, except you don't get one item at a time, but all of them. So name the method after Queue's or Stack's method?



  • The Real WTF is it seems like a longwinded way of writing:

     ArrayList someVar = new ArrayList();

     Why on earth would you write:

    ArrayList someVar = someObj.getChokes();

     ???

    [EDIT]  I've just seen it....doh!



  • @tonio said:

    @joelkatz said:

    So the WTF is the method name. Though it's hard to think of a better method name that's not awkward. takeChokes? flushChokes?

    removeChokes()? popChokes()? It's working as a Queue or a Stack, basically, except you don't get one item at a time, but all of them. So name the method after Queue's or Stack's method?

     

    PerformHeimlich()  !!



  • @joelkatz said:

    I suspect this is code that accumulates the list of pees to choke (rather than the list of peers that are choked) and the only time the list is retrieved is to send it. So the WTF is the method name. Though it's hard to think of a better method name that's not awkward. takeChokes? flushChokes?
     

    dumpChokes()!



  • @campkev said:

    @tonio said:

    @joelkatz said:

    So the WTF is the method name. Though it's hard to think of a better method name that's not awkward. takeChokes? flushChokes?

    removeChokes()? popChokes()? It's working as a Queue or a Stack, basically, except you don't get one item at a time, but all of them. So name the method after Queue's or Stack's method?

     

    PerformHeimlich()  !!

    ejectBolus() because it coughs up the chokes!  Then PerformHeimlich would be a method in a friend class that invokes ejectBolus :-P

Log in to reply