1
\$\begingroup\$

I have a collection which, after the code has ran, looks something like the following:

[
  { title: 'alpha', lines: [ { *some object* } ] },
  { title: 'bravo', lines: [ { *some object* } ] },
  { title: 'charl', lines: [ { *some object* } ] },
  { title: 'delta', lines: [ { *some object*, *some object 2*, *some object 3* } ] m},
  { title: 'echoo', lines: [ { *some object* } ] },
]

The way that I'm doing this is:

  1. Check if there is an object in the outer array that has a matching title
    • If it doesn't exist, create that object with the title and lines
  2. Check if inside lines there is an object that exists at the specific supplied index
    • If it doesn't exist, create the object at that index
    • Otherwise take the existing object at that index and append val to a_or_b

This method will get called multiple times per group i.e. 4 times for alpha, 4 times for bravo, and 4 times for each line in delta. There is currently one group (but there may be more in the future) which has more than one object inside the lines.

def data_with(data, val, group, date, nom, a_or_b, line_no = 0)
  # add this group if it doesn't exist
  if data.find { |x| x[:title] == group }.nil?
    data << { title: group, lines: [] }
  end

  # if the line at the line_no doesn't exist, create it
  if data.find { |x| x[:title] == group }[:lines][line_no].nil?
    data.find { |x| x[:title] == group }[:lines][line_no] = {
      date: date,
      nom: nom,
      a_or_b.to_sym => val
    }
  else # add to the value of the line at the line_no index
    data.find { |x| x[:title] == group }[:lines][line_no][a_or_b.to_sym] += val
  end
end

What are some ways I can improve this method?

\$\endgroup\$
0
\$\begingroup\$

If the objects in the "outer array" was a class then 2 of the ifs go away because of the context of a constructor. Return after creating it, do not fall through to the following ifs

Create a local variable and do only 1 data.find at the start of data_with

If execution gets to that final if then you know it is not a new just created object.

\$\endgroup\$
0
\$\begingroup\$

In general I think something feels off with your data structure and the naming of your parameters / variables is extremely difficult to understand (e.g. what is an a_or_b?).

Anyway, you could structure your code a bit more object oriented and extract helper methods. I don't know the full context of your application so this might be overkill. To provide some food for thoughts here is an example:

input = [
  { title: 'alpha', lines: [ { a_or_b: 1 } ] },
  { title: 'bravo', lines: [ { a_or_b: 1 } ] },
  { title: 'charl', lines: [ { a_or_b: 1 } ] },
  { title: 'delta', lines: [ { a_or_b: 1 } ] },
  { title: 'echoo', lines: [ { a_or_b: 1 } ] },
]

class Collection
  class Entry
    def initialize(data)
      @data = data
    end

    def ==(other)
      @data[:title] == other
    end

    def update_or_create_line_by(index:, date:, nom:, a_or_b:, value:)
      update_line_by(index: index, value: value, a_or_b: a_or_b) ||
        create_line_by(index: index, date: date, nom: nom, a_or_b: a_or_b, value: value)
    end

    def create_line_by(index:, date:, nom:, a_or_b:, value:)
      lines[index] = {
        date: date,
        nom: nom,
        a_or_b.to_sym => val
      }
    end

    def update_line_by(index:, value:, a_or_b:)
      return unless lines[index]

      lines[index][a_or_b.to_sym] += value
    end

    private

    def lines
      @data[:lines]
    end
  end

  def initialize(data)
    @data = data.map { |entry| Entry.new(entry) }
  end

  def find_or_create(title:)
    find_by(title: title) || create_by(title: title)
  end

  def find_by(title:)
    @data.find { |entry| entry == title }
  end

  def create_by(title:)
    entry = Entry.new(title)
    @data << entry
    entry
  end
end

def data_with(data, val, group, date, nom, a_or_b, line_no = 0)
  collection = Collection.new(data)
  entry = collection.find_or_create(title: group)
  entry.update_or_create_line_by(index: line_no, date: date, nom: nom, a_or_b: a_or_b, value: val)
end

data_with(input, 1, 'alpha', Time.now, 'nom', 'a_or_b', 0)
puts input
```
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.