Reduce calls to `#entity` using local variables

[ashmaroli]

Slight improvement to #educate_quotes by dropping unnecessary calls to #entity

[agriffis]

Hi @ashmaroli

Thanks for the PR. This trades calls for temporary variables. It's not a clear win. Could you explain your reasoning?

[ashmaroli]

It's not a clear win.

You're right. Benchmarks don't report much of a gain. (The minor gain tilts towards this patch, albeit not so discernible) My rationale for submitting the PR anyways is reduced "code-smell" due to reduced calls to a method with the same argument.

[ashmaroli]

I also planned on submitting a PR assigning the various regexps to constants so that they are not created per call to #educate_quotes

[agriffis]

You're right. Benchmarks don't report much of a gain. (The minor gain tilts towards this patch, albeit not so discernible) My rationale for submitting the PR anyways is reduced "code-smell" due to reduced calls to a method with the same argument.

For code smell, I'd be more interested in accessing @entities directly rather than creating temporary vars.

I also planned on submitting a PR assigning the various regexps to constants so that they are not created per call to #educate_quotes

I'm more interested in code clarity and maintainability than performance unless there's a significant measurable improvement.

[ashmaroli]

I'd be more interested in accessing @entities directly

calling @entities[:single_left_quote] multiple times is functionally identical to entity(:single_left_quote) being called multiple times.. I don't see how that's an improvement in code-smell

[agriffis]

no you're right, it doesn't improve code smell. It just removes an unnecessary indirection.

But to me, this is bad code smell:

foo = something(:foo)
bar = something(:bar)
baz = something(:baz)
qux = something(:quz)

It's just a lot of literal repetition to pull data into same-name temporaries. Easy to introduce errors since it's manual instead of programmatic.