rubyで、テーブルのデータの合計値を算出する

Ruby

Last updated at 2023-12-16Posted at 2022-10-05

課題

テーブルのセルの[列&行]を合算する

実装方法

テーブルを作成
行の合計値を出す
列の合計値を出す
テーブルに、行の合計値を足す
テーブルに、列の合計値を足す

table = [
    [9, 85, 92, 20],
    [68, 25, 80, 55],
    [43, 96, 71, 73],
    [43, 19, 20, 87],
    [95, 66, 73, 62]
]

line_sum = table.map(&:sum)
p line_sum
column_sum = table.transpose.map(&:sum)
p column_sum

p table.concat([column_sum])
table_sum = table.each_with_index{|line,index|
    line.push(line_sum[index])
}
p table_sum

結果

# 行の合計
[206, 228, 283, 169, 296]
# 列の合計
[258, 291, 336, 297]
# 列を合計したテーブルの値
[[9, 85, 92, 20], [68, 25, 80, 55], [43, 96, 71, 73], [43, 19, 20, 87], [95, 66, 73, 62], [258, 291, 336, 297]]
# 最終的なテーブルの値
[[9, 85, 92, 20, 206], [68, 25, 80, 55, 228], [43, 96, 71, 73, 283], [43, 19, 20, 87, 169], [95, 66, 73, 62, 296], [258, 291, 336, 297, nil]]

以上のようなコードになるのですが、

ダメな所
もっと効率の良い書き方

をレビューして頂けないでしょうか？

レビューを受けての変更後

@scivolaさんのコメントを考慮して変更したコード

このコードだと右下の角の値（合計の合計）が nil になりますね
最初に与えられた table の値を破壊してしまうのは，要件によってはまずいですね
column_sum から配列を作って concat しなくても

origin_table = [
    [9, 85, 92, 20],
    [68, 25, 80, 55],
    [43, 96, 71, 73],
    [43, 19, 20, 87],
    [95, 66, 73, 62]
]
# 元のテーブルを壊さないように、複製する
dup_table = Marshal.load(Marshal.dump(origin_table))

line_sum = dup_table.map(&:sum)
p line_sum
column_sum = dup_table.transpose.map(&:sum)
p column_sum

dup_table << column_sum

sum_table = dup_table.each_with_index{|line,index|
    line << line_sum[index]
}

# 最終行、最終列がnilになっている事を確認
pp sum_table[-1][-1]
puts
# 最終行のnilを取り除くとsum出来る
sum_table[-1][-1] = sum_table[-1].compact.sum

pp sum_table
puts
# 元のテーブルに変更がない事を確認
pp origin_table

結果

テーブルの複製に関して

@scivola さんより指摘を頂き、テーブルを複製する際は、Marshalクラスを使うのではなくて、
Arrayクラスのdupを使う方が有用である事がわかりました
https://docs.ruby-lang.org/ja/latest/method/Object/i/clone.html

dupでコピーをする際は、深さを意識してコピーをする必要があります

clone や dup はオブジェクト自身を複製するだけで、オブジェクトの指している先(たとえば配列の要素など)までは複製しません。これを浅いコピー(shallow copy)といいます。

なので、dupでコピーする際は、配列の要素に対して、dupを使う必要があります

[例] 深いコピー ※配列の要素に対して、dupを実行する

origin_table = [
    [9, 85, 92, 20],
    [68, 25, 80, 55],
    [43, 96, 71, 73],
    [43, 19, 20, 87],
    [95, 66, 73, 62]
]
# 元のテーブルを壊さないように、複製する
dup_table = origin_table.map{ |row| row.dup } # 深い
# dup_table = origin_table.dup # 浅い

# 同じオブジェクトか確認
p(origin_table.equal?(dup_table))

line_sum = dup_table.map(&:sum)
p line_sum
column_sum = dup_table.transpose.map(&:sum)
p column_sum

dup_table << column_sum

sum_table = dup_table.each_with_index{|line,index|
    line << line_sum[index]
}

[深いコピー]結果

[例] 浅いコピー ※配列の要素に対しては、dupをしない

origin_table = [
    [9, 85, 92, 20],
    [68, 25, 80, 55],
    [43, 96, 71, 73],
    [43, 19, 20, 87],
    [95, 66, 73, 62]
]
# 元のテーブルを壊さないように、複製する
# dup_table = origin_table.map{ |row| row.dup } # 深い
dup_table = origin_table.dup # 浅い
# 同じオブジェクトか確認
p(origin_table.equal?(dup_table))

line_sum = dup_table.map(&:sum)
p line_sum
column_sum = dup_table.transpose.map(&:sum)
p column_sum

dup_table << column_sum

sum_table = dup_table.each_with_index{|line,index|
    line << line_sum[index]
}

# 最終行、最終列がnilになっている事を確認
pp sum_table[-1][-1]
puts
# 最終行のnilを一度取り除くとsum出来る
sum_table[-1][-1] = sum_table[-1].compact.sum

pp sum_table
puts
# 元のテーブルに変更がない事を確認
pp origin_table

[浅いコピー]結果 ※要素に対してはdupを行なっていないので、コピー元が影響を受けている

簡単に実装できる事が判明、、

@obelisk68　さんのコメント通り試したら、簡単に実装出来ました、、

table = [
    [9, 85, 92, 20],
    [68, 25, 80, 55],
    [43, 96, 71, 73],
    [43, 19, 20, 87],
    [95, 66, 73, 62]
  ]
  # mapで行を回して、合計値を足す
  tmp = table.map { |row| row + [row.sum] }
  puts 
  # transposeで行&列を入れ替えて合計し、元に戻す
  pp result = tmp.transpose.map { |col| col + [col.sum] }.transpose

個人的反省点

経験値が低すぎるのかも知れない、、
→ コードを読む量　& 書く量が少なくて、実装方法が分かってない

上の解決策

たくさんコードを書いたり、読んだりする経験を増やす
paiza等で、プログラミングの問題を解く
→有用じゃないかと思ってる。
実際にコードを書いて実装してみないと
・分からない所
・分かってない所
に気づけないので

テストコード付きで書き直してみた

テストがないので、コードを変更する度に、動作確認するのが大変だった。
テストも出来るように、書き直してみました。
※こちらもレビュー頂けるとありがたいです
https://github.com/syo-tokeshi/sum-table-by-ruby

実行クラス

class Table
    @@table = [
        [9, 85, 92, 20],
        [68, 25, 80, 55],
        [43, 96, 71, 73],
        [43, 19, 20, 87],
        [95, 66, 73, 62]
    ]

    def self.get_table_data
        @@table
    end

    def self.genarate_table
        add_line_sum
        column_sum
        @@table
    end

    # mapで行を回して、合計値を足す
    def self.add_line_sum
        @@table = @@table.map { |row| row + [row.sum] }
    end
    
    # transposeで行&列を入れ替えて合計し、元に戻す
    def self.column_sum
        @@table = @@table.transpose.map { |col| col + [col.sum] }.transpose
    end
end

# pp Table.genarate_table

テストクラス

require './table.rb'
require 'minitest/autorun'

class TableTest < Minitest::Test

  # テーブルのデータが有効か確認
  def test_valid_table_data
    assert Table.get_table_data
  end

  # 行の合計を計算出来るか
  def test_add_line_sum
    assert_equal 206,Table.add_line_sum[0][-1]
  end

  # 列の合計を計算出来るか
  def test_column_sum
    assert_equal 258,Table.column_sum[-1][0]
  end

  # 合計したテーブルが作れることを確認
  def test_genarate_table
    assert Table.genarate_table
  end

end

テストに通れば、4項目クリア出来ます

% ruby "/Users/admin/tokeshi_free_space/test_table.rb"
Run options: --seed 22557

# Running:

....

Finished in 0.001053s, 3798.6693 runs/s, 3798.6693 assertions/s.

4 runs, 4 assertions, 0 failures, 0 errors, 0 skips

参考文献

minitest公式サイト
https://www.rubydoc.info/gems/minitest

配列のコピーについて
https://mistyrinth.hatenablog.com/entry/2019/03/04/173445

You get articles that match your needs
You can efficiently read back useful information
You can use dark theme

What you can do with signing up