These are chat archives for ManageIQ/manageiq/performance

17th
Mar 2016
Keenan Brock
@kbrock
Mar 17 2016 13:08
This is a common pattern. Is there an alternative form:
fields(message).collect { |f| f['name'] if f['aetype'] == 'state' }.compact
Matthew Draper
@matthewd
Mar 17 2016 13:11
fields(message).select { |f| f['aetype'] == 'state' }.pluck('name') ?
Keenan Brock
@kbrock
Mar 17 2016 13:11
do you think that would be more performant? / less memory?
and more readable - worth hitting the code base? probably have ~50-100 of these
Matthew Draper
@matthewd
Mar 17 2016 13:12
No perf difference, I don't think
Keenan Brock
@kbrock
Mar 17 2016 13:13
I find in an iterator, I often want to say "skip"
Matthew Draper
@matthewd
Mar 17 2016 13:13
NB they're only functionally the same if f['name'] is always set / never nil
Keenan Brock
@kbrock
Mar 17 2016 13:13
yea
thnx
sometimes we throw in an unneeded flatten, other times we just return f (so it is probably just a select)
ok - thanks
Matthew Draper
@matthewd
Mar 17 2016 13:14
I do generally consider select+map to be clearer than map { if }.compact
Keenan Brock
@kbrock
Mar 17 2016 13:15
yea, I was hoping you were going to tell me there was a Enumerable#select_if_map() ;)
e.g.: translate_values
always new nuggets hiding in there
Chris Arcand
@chrisarcand
Mar 17 2016 13:15
You can also reduce, the thing you say you never use ;)
a.each_with_object([]) { |f, m| m << f if f['aetype'] == 'state’ }
Keenan Brock
@kbrock
Mar 17 2016 13:16
yea - was one of my alternative solutions
Oleg Barenboim
@chessbyte
Mar 17 2016 13:16
wow - that's really hard to read
Keenan Brock
@kbrock
Mar 17 2016 13:16
agreed
Matthew Draper
@matthewd
Mar 17 2016 13:16
fields(message).flat_map { |f| f['aetype'] == 'state' ? f['name'] : [] }
Chris Arcand
@chrisarcand
Mar 17 2016 13:16
Heh :D I don’t think it’s any better, no.
Keenan Brock
@kbrock
Mar 17 2016 13:17
matthewd - yea. had that one too. there are a bunch of alternatives
I had select{}.collect(&:X)but I like the select{}.pluck(:X) - good one
Matthew Draper
@matthewd
Mar 17 2016 13:20
FYI note those aren't the same… pluck is collect + #[]
Keenan Brock
@kbrock
Mar 17 2016 13:20
@chessbyte I really like ManageIQ/manageiq#7260
but I introduced a dumb syntax error in there and the tests were green.
intimidated in writing a spec for the two vim_performance_analysis methods I cal
@matthewd aah - of course. thanks. didn't see that
Matthew Draper
@matthewd
Mar 17 2016 13:21
I don't think there's a "nice" option, just because it's doing two fairly separate things
The real answer, of course, is "use real objects instead of hashes".. or in this case, meta-hashes :neutral_face:
Keenan Brock
@kbrock
Mar 17 2016 13:21
good point. I guess map reduce is 2 words because it is a common pattern, but they are 2 steps
Chris Arcand
@chrisarcand
Mar 17 2016 13:23
¯\_(ツ)_/¯ I think this is a pretty standard reduction. In Ruby we just tend to break them apart often. (because people think the memo in reduce or each_with_object is ugly ;))
Keenan Brock
@kbrock
Mar 17 2016 13:26
@chrisarcand I really like flat_map. You can return [] to signify skip this. that is very easy to read and allows a simple map and reduce in a single step
Chris Arcand
@chrisarcand
Mar 17 2016 13:30
Ahh I never thought about that side effect of purposely passing empty. [1,2,3].flat_map { |x| x.even? ? [] : x } # => [1,3] neato.
Marcel Hild
@durandom
Mar 17 2016 13:33
i hope you put those method chains in small methods with a descriptive name :)
Keenan Brock
@kbrock
Mar 17 2016 13:40
@durandom the method names are longer than the code :)
Marcel Hild
@durandom
Mar 17 2016 13:41
haha, which is ok, if it serves as documentation
Keenan Brock
@kbrock
Mar 17 2016 13:44
def method_i_am_not_really_sure_the_name_and_sorry_because_i_think_ruby_has_this_built_in_but_too_lazy_to_look_it_up_and_sorry_too_lazy_to_find_a_shorter_name_but_read_comments_because_they_are_basically_a_sorry_as_well()
  @x.map(&:y)
end
Chris Arcand
@chrisarcand
Mar 17 2016 13:46
@durandom As long as those methods are private...
Keenan Brock
@kbrock
Mar 17 2016 13:48
@chrisarcand no need. they aren't called by anyone anyway
I had a coworker once say "a comment is just an apology for not writing better code"
I agree 85%
Chris Arcand
@chrisarcand
Mar 17 2016 13:48
O_o That’s the point, IMO. If you don’t, somebody will use it.
Keenan Brock
@kbrock
Mar 17 2016 13:48
sorry for snark
lol
Marcel Hild
@durandom
Mar 17 2016 13:50
depends what 'better' is. I mean sometimes you can easily get rid of the comment by moving the code to method and naming it descriptive
Keenan Brock
@kbrock
Mar 17 2016 13:51
yea, sometimes a method name or variable name serves as documentation.
other times, just "deleting code" really serves by simplifying the algorithm and making it evident
(yes, you can go overboard. not suggesting converting it into perl...)