I'm curious why the quotes are modified in-place here, rather than being returned. I find it easier to understand what a function does if it doesn't rely upon mutating the parameters, and the types become much more informative as well.

I also don't think any of these new values should be attached to the quote object before they're used either. This strategy of keeping state around for later has left us with a lot of unused state that we had forgotten to clean up. Best to add it only when we being actually using it.

In my understanding, problems with mutating in place arise when the mutated data is accessed by many different contexts over its lifetime. In this instance, the mutation occurs at a single time and place, namely when _addTopQuoteAndTradeData is called in fetchAndSetQuotes.

I'd suggest that we return the modified quotes object from _addTopQuoteAndTradeData, while still modifying it in place. This would grant us the type benefits without having to reinstantiate the quotes object.

If we want to also reinstantiate the quotes object in _addTopQuoteAndTradeData as a matter of principle, I'm also fine with that, although I don't think it does us any good in this case.

problems with mutating in place arise when the mutated data is accessed by many different contexts over its lifetime.

Nah I wasn't referring to anything like this. I meant it was hard to understand. Mutation makes the code hard to read.

I expect a function's type signature to give some hint as to what it's doing - stuff goes in, stuff comes out. It makes tracing the code easier as well. I would never have expected this to mutate the quotes before reading this doc comment. This is particularly problematic when reading where this function is used, as it wouldn't occur to me to read the definition of every function invoked just to follow what was going on with the quotes. I would see that it returned the ID of the top quote, and assume that's all it did.

We also lose any opportunity to describe the operation with types if we rely on mutation. With proper parameter and return types, we can express using the type system that these properties were not present before, but may be present afterwards.

We also lose any opportunity to describe the operation with types if we rely on mutation.

I was suggesting that we can mutate the quotes in place and return them (altering the function's return type) to get this benefit:

we can express using the type system that these properties were not present before, but may be present afterwards.

I'd suggest that we return the modified quotes object from _addTopQuoteAndTradeData, while still modifying it in place. This would grant us the type benefits without having to reinstantiate the quotes object.

Ah, right, this sounds fine. Sorry I only meant to take issue with the function signature, not the mutating of the parameter itself. I guess I don't feel strongly one way or the other about whether the parameter is mutated in this case, since it's a private function used in one place - I explained that poorly.

Then we are in agreement, I shall alter the function signature!