activegraph icon indicating copy to clipboard operation
activegraph copied to clipboard

Refactor HasnN#association_proxy

Open yourpalal opened this issue 5 years ago • 1 comments

Clean up and comment the HashN#association_proxy method which was pretty convoluted. Functionally equivalent to #1530, so this is a style-only fix, once that PR has been merged.

  • add comments
  • reduce indentation with an early next when @source_proxy_result_cache is falsey
  • rename hash variable to proxy_hash_key
  • rename cache variable to association_query_results
  • rename result_cache to source_result_cache
  • create proc on its own line, with simpler code
  • instead of using inject to iterate and also find the return value, use each to iterate while setting association_proxy_cache for all the objects, then use association_proxy_cache to get the right return value.

Technically, this also fixes #1529, but there is another PR that fixes it with a smaller change (#1530)

yourpalal avatar May 03 '19 14:05 yourpalal

BTW, CI failed due to rubocop issues:

Offenses:
lib/neo4j/active_node/has_n.rb:212:5: C: Metrics/AbcSize: Assignment Branch Condition size for association_proxy is too high. [18.06/18] (http://c2.com/cgi/wiki?AbcMetric)
    def association_proxy(name, options = {}) ...
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
lib/neo4j/active_node/has_n.rb:212:5: C: Metrics/MethodLength: Method has too many lines. [17/15] (https://github.com/bbatsov/ruby-style-guide#short-methods)
    def association_proxy(name, options = {}) ...
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

2 lines too many and 0.06 too many ABC points :/ I guess splitting into 2 methods would do better?

yourpalal avatar Jul 02 '19 19:07 yourpalal