On Sun, Jul 3, 2016 at 9:10 PM, Benjamin Mahler <[email protected]> wrote: > To clarify, are you ok with the removeSlave example? It seems to fit your > criteria.
I think `removeSlave` is poorly named, for similar reasons -- I just talked about `update` in my email for brevity. > Usually with this kind of email we need concrete suggestions for > improvement. My email included the following, which I think is pretty concrete: *** I'd like to propose that we avoid naming functions in this style: if two functions do fundamentally different things or should be invoked in very different circumstances, we should avoid giving them the same name. We should use overloading when two variants of a function do basically the same thing but differ slightly in the parameters they accept. *** We can certainly debate the specifics of whether/how to rename particular functions, but I think the bigger question is whether we want to endorse using overloading to differentiate between functions that do fundamentally different things. > With this I don't think it's as bad as you've described in terms of being > able to intuit behavior: > > sorter->update(client, weight); > sorter->update(slaveId, newTotal); > sorter->update(client, slaveId, oldAllocation, newAllocation); > > That being said, if there are more helpful function names let's make some > suggestions! The obvious alternative here seems to be verbose names that > repeat the arguments? > > sorter->update_client_weight(client, weight); > sorter->update_slave_total(slave, total); > sorter->update_client_allocation(client, slaveId, oldAllocation, > newAllocation); > > We tend to avoid this pattern as well, because it leads to redundancy. I would happily accept a little more redundancy for these examples, because I think it improves the clarity of the code. We tend to favor clarity and redundancy over brevity in several other situations (e.g., variable names must be entire words, using `load` and `store` for atomics rather than operator overloading). For the specific examples above, I think the longer names (e.g., `update_client_weight`) are an improvement. There's also a compromise (adding back the fourth variant of `update` which is a private function): sorter->updateWeight(client, weight); sorter->updateTotal(slaveId, total); sorter->updateAllocation(client, slaveId, oldAllocation, newAllocation); sorter->updateShare(client); Neil
