2 ms·
Ones is more explicit and the other is more terse but I honestly can’t understand the argument that one version is more likely to cause bugs than the other. Is
by meheleventyone 7y ago
Ones is more explicit and the other is more terse but I honestly can’t understand the argument that one version is more likely to cause bugs than the other. Is this a simple version for illustration and if so what do the more complicated versions look like?
- lowdest 7y agoThe point is to not modify the object that you are looping over from within the loop. It is very easy to create bugs this way.
- Supermancho 7y ago> not modify the object that you are looping over from within the loop That isn't happening. The loop is over an integral range (1..5 in both examples). That integer is being hashed in the example and the hash is being used for a SIDE EFFECT. Internally, the select() is doing the exact same thing, but there is no label on the value and it's being returned out of the ostensible container function. This is not a good example of why this pattern is bad nor of why side effects are bad.
- backslash_16 7y agoIf I have read his example correctly - he's not modifying the object he is looping over. car_ids is externally defined and he is using a loop to exhaustively test other objects and take an action on them, which is adding them to the externally defined array. I do agree with what you are saying, don't modify the enumerable you loop over. IIRC C# is pretty safe in that it doesn't allow modifications to the enumerable inside of a for loop or foreach loop. I think it's interesting people say functional constructs like map don't involve looping. They do, it's just hidden from the caller of those functions.
- AnimalMuppet 7y agoThe argument is that the more explicit version has more things that you have to explicitly specify, and some of those things have to be consistent with each other. You can mess it up by getting the parts out of sync. So when you don't need the detailed control, the terse version has fewer things you have to say, so fewer things that you can get wrong. On the other hand, if you do need the detailed control, the terse version doesn't let you have it, so you have to use the verbose version.
- meheleventyone 7y agoI don’t buy that, the explicit version has more syntax to type but no less decisions to make. In the terse version you still need to map and select, you have to make those decisions, you have to do them in the right order and you still have to get the criteria in the lambda correct. One hides the loop (potentially doing more work because of it) and one doesn’t.
- swsieber 7y agoIn the explicit version you need to make sure you push car_id instead of i. There are more opportunities for decisions like that the longer the logic chain. The big impact as I see it is that it removes non-important bits and keeps you from messing those up, namely picking the right variable and indexing. Each new lambda in the chain is scoped to it's inputs, not all previous inputs in the iteration.
- meheleventyone 7y agoI think this falls under getting the lambda and choice of function right. Likewise more complex expressions are easy to get wrong. There are implementation details to both you need to nail. Is it better to fail in referencing the wrong variable or calling the wrong function with the wrong (but valid) input? Which is to say if you get the implementation details wrong you’ve got the implementation wrong and notice very quickly with any halfway reasonable test.
- Jtsummers 7y agoThere's no question which order to perform the map and select in the given example, there's no decision to make. You can't do the select until after the map because you're selecting based on the results of the map. If it weren't ruby, it would be obvious by the types in a language with a static type system. Of course the same is true in the for-loop example. But overall, there are more things to specify in the for-loop: let car_ids = []; for (let i = 1; i <= 5; i++) { const car_id = `car-${i}`; if (car_is_empty(car_id)) { car_ids.push(car_id); } } return car_ids; Specify: - storage location for result - loop variable - loop variable initial value - terminating condition (is it < or <=?) - increment - temp variable for producing ids - test condition for filtering - how to add new item to result return (1..5) .map{|n| "car-#{n}"} .select{|m| car_is_empty(m)} Specify: - min value of range - max value of range (are ranges inclusive or exclusive?) - how to generate id - how to query Now, this is a trivial example. I wouldn't argue that either way is clearly better than the other in this particular case. But, in my experience and opinion, the latter is much better as things become more complex.
- pornel 7y agoYeah, the examples are too simple. Things can get tricky if you start modifying the container you're iterating, especially with deletions. OTOH iterators can make this impossible (e.g. through borrowing in Rust) or at least detect "iterator invalidation" at runtime. In certain programs it's also easy to modify the container by accident, e.g. if you call a function that triggers an event that indirectly modifies your container. If you do arithmetic on iteration variable, especially if the loop body has conditionals, then it's harder to follow the code, and easier to make off-by-one errors or infinite loops. Not every loop is clearer with iterators, but some idioms like `.filter()` or `.any()` are clearer and even shorter to write.
- meheleventyone 7y agoI definitely agree with some of that but it feels like you’re bringing in a bunch of arguments not made in the original post rather than showing the arguments made need more complex examples.