News for hackers. Stuff that matters.
Re:Code Smells: Iteration by jrs95 on Wednesday August 23, 2017 @11:15AM (#15082122)
It's obvious to some of us, but not all of us. I see things way worse than this on a fairly regular basis.
Re:Code Smells: Iteration by jnty on Wednesday August 23, 2017 @11:28AM (#15082257)
I guess - but I feel like you'd call it an "error" rather than a "smell". Better to just teach people data structures rather than which constructs they should be afraid of. Approaching it "sideways" like this just leads to people going "I heard that loops can be bad, so I copied and pasted that line 25 times instead".
Re:Code Smells: Iteration by jayd16 on Wednesday August 23, 2017 @11:52AM (#15082500)
Its a smell and not an error because it works perfectly fine and it actually is perfectly fine for now because the n for that linear search is low.
Re:Code Smells: Iteration by rootlocus on Wednesday August 23, 2017 @11:29AM (#15082269)
Sets have a very specific use case: they only contain unique values and (unless using a specific implementation) they don't have any defined order. Switching from arrays to sets just to get rid of an iteration doesn't sound like fixing a smell to me. Also, insertion, iteration and memory usage are less efficient for a set.
Re:Code Smells: Iteration by JackFr on Wednesday August 23, 2017 @12:42PM (#15082969)
Importantly Sets have different semantics than Lists. Creating a set is an information losing process. A list converted to set cannot be recovered, but a set converted to a list is easily recovered.

I don't think this is handled well in TFA. Because of some dubious iteration at the call site, the author changes the semantics of getLoadNames(), blithely assuming that duplicates should not be allowed and order is not important.

The author mentions that the function is called at two other places. For all we know the original author was aware of Sets but chose List because it more correctly matched the use of the structure.