回答編集履歴

3

ちょっと追記

2016/09/27 12:16

投稿

raccy
raccy

スコア21735

test CHANGED
@@ -95,3 +95,7 @@
95
95
  * マジックナンバーを使用しないでください。最初に`hrt_size = $hrt.size`などし、`9`は`hrt_size - 2`、`10`は`hrt_size - 1`などと表現してください。
96
96
 
97
97
  * 一つのメソッドにおいて、行数が多く、また、インデントが深すぎです。メソッドの分離を検討してください。
98
+
99
+
100
+
101
+ 色々書いてしましたが、私が10年以上前に最初に書いていたRubyのコードもこんな物でした。焦らずに、少しずつ慣れていけば良いのですから。

2

2\.3\.xに対応

2016/09/27 12:16

投稿

raccy
raccy

スコア21735

test CHANGED
@@ -1,16 +1,30 @@
1
- Ruby 2.4.0-preview2 (2.3.x下で動きません)
1
+ Ruby 2.3系に対応しました。lambda_driverとactivesupportが必要です(2.4.0上の場合activesupportは不要)。`gem instnall lambda_driver activesupport`でインストールしてから実行してください。
2
+
3
+
4
+
5
+ 主な変更点: `chunk`ではなく`slice_before`で切るようにしました。lambda_driverで関数合成するようにしました。`Enumerable#sum`は2.4.0からのため、2.3系についてはactivesupportの実装を使うようにしました。
6
+
7
+
2
8
 
3
9
  ```Ruby
10
+
11
+ # frozen_string_literal: true
12
+
13
+ require 'lambda_driver'
14
+
15
+ require 'active_support/core_ext/enumerable' if RUBY_VERSION < '2.4.0'
16
+
17
+
4
18
 
5
19
  def yyyy(hrt)
6
20
 
7
21
  hrt.each_with_index
8
22
 
9
- .chunk { |v, _i| v.zero? }
23
+ .slice_before(&:first >> :zero?)
10
24
 
11
- .find { |r, a| !r && -> (x) { x.sum == 4 && x != [2, 2] }[a.map(&:first)] }
25
+ .find { |a| -> (x) { x.sum == 4 && x != [0, 2, 2] } < a.map(&:first) }
12
26
 
13
- &.last&.inject(0) do |r, vi|
27
+ &.inject do |r, vi|
14
28
 
15
29
  case vi.first
16
30
 
@@ -18,7 +32,7 @@
18
32
 
19
33
  when 2, 4 then break vi.last
20
34
 
21
- when 3 then r
35
+ else r
22
36
 
23
37
  end
24
38
 
@@ -42,4 +56,42 @@
42
56
 
43
57
 
44
58
 
59
+ バージョン比較の部分は2.10.0とか出たときは対応できないバグがありますが、その前に3.0.0が出ることを期待して…。
60
+
61
+
62
+
63
+ ---
64
+
65
+ 【質問のコードのレビュー】
66
+
67
+
68
+
69
+ すいません、全く違う作りにしてしまった理由というか、質問のコードについて指摘事項を羅列します。
70
+
71
+
72
+
73
+ * Ruby 1.9.4は既にサポートされていません。バグが原因で動かない場合や脆弱性が発生した場合、誰も助けることはできません。サポートされたRubyのバージョンを使用してください。現在サポートされているのは、2.3系、2.2系、2.1系(ただしセキュリティ修正のみ)です。
74
+
75
+ * グローバル変数は、グローバル変数を使わなければ実現できない等のどうしても必要な理由が無い限り、使用しないでください。
76
+
77
+ * ループは一方向のみにしてください。添字のiが増えたり減ったりすることはわかりにくく、バグの温床になります。高速なアルゴリズムを実現するなどの目的が無い限り、増える方向、または、減る方向のみに絞ってください。
78
+
79
+ * 添字のループは`Integer#times`や`Range#each`を使ってください。`while`は数がわからないと言う事情が無い限り、極力避けてください。かといって、代わりに`for in`を使うことは避けてください。`for in`は`each`との違いが理解できない限り、使用しないでください。`for in`で無ければならないことはほとんどありません。
80
+
81
+ * 配列を順番に見るときは、`Array#each`を使ってください。添字が必要であれば`Array#each_with_index`を使ってください。
82
+
83
+ * `i = i + 1`は`i += 1`と書いてください。他の部分も、複合代入演算子を使用してください。
84
+
45
- もうちょっと、工夫したい。合成関数とか使って(Rubyど)
85
+ * 同じ行式が続くので無ければ、`then`は不要です
86
+
87
+ * 何もしない`else`は不要です。
88
+
89
+ * `if X == a then ... elsif X == b then ... elsif X == c then ... else ... end`と言う形の場合は、case文を使用してください。
90
+
91
+ * 単純な添字ループであれば`i`でもかまいませんが、複雑な処理になる場合は、何を意味する物かを示すように変数名を与えてください。`j`についても同様です。また、`cnt`はカウンタと言うよりも数の和です。`total`などにしたほうがわかりやすいでしょう。
92
+
93
+ * 配列の長さが固定でない場合を想定してください。
94
+
95
+ * マジックナンバーを使用しないでください。最初に`hrt_size = $hrt.size`などし、`9`は`hrt_size - 2`、`10`は`hrt_size - 1`などと表現してください。
96
+
97
+ * 一つのメソッドにおいて、行数が多く、また、インデントが深すぎです。メソッドの分離を検討してください。

1

findつかえば一発じゃ無いか!何をオレは…

2016/09/27 12:03

投稿

raccy
raccy

スコア21735

test CHANGED
@@ -6,11 +6,11 @@
6
6
 
7
7
  hrt.each_with_index
8
8
 
9
- .chunk { |v, _i| v.zero? }.reject(&:first).map(&:last)
9
+ .chunk { |v, _i| v.zero? }
10
10
 
11
- .select { |a| -> (x) { x.sum == 4 && x != [2, 2] }[a.map(&:first)] }
11
+ .find { |r, a| !r && -> (x) { x.sum == 4 && x != [2, 2] }[a.map(&:first)] }
12
12
 
13
- .first&.inject(0) do |r, vi|
13
+ &.last&.inject(0) do |r, vi|
14
14
 
15
15
  case vi.first
16
16