知らないと恥ずかしいコードレビューで指摘されがちなポイント14選

この記事はNuco Advent Calendar 2022の13日目の記事です

はじめに

私は情報系の学部に通う大学４年生です。大学でプログラミングを学んだことをきっかけに、プログラミングを使用した実際の業務に取り組んでみたいと思いました。そして、株式会社Nucoさんで機会をいただき、現在インターン生として実務に参加させていただいています。

自分のように、プログラミングを学び、「実務の経験が積みたい」「インターンに参加してみたい」という方はたくさんいらっしゃるかと思います。この記事では自分が実際にインターン生として実務に参加し、コードレビューで指摘されたポイントを紹介します。

難易度、頻出度の目安を★の数で示しています。
・難易度・・・それぞれの項目で指摘されないようなコードを書く難しさ。
・頻出度・・・それぞれの項目のミスの起きやすさ。

難易度(低→高)、頻出度(高→低)の順番で紹介していきます。

※例はPythonですが、ほとんどは他の言語にも当てはまる話です。

指摘されがちなポイント

１.複数の機能がある関数

難易度：★☆☆　頻出度：★★★

１つの関数・メソッドに複数の機能がある場合、その関数・メソッドがどんな役割を果たすのかがわかりにくくなってしまいます。これは保守性・可読性の低下につながります。

１つの関数・メソッドには機能を１つだけ持たせるようにしましょう。

# ❌ 合計を計算する機能と偶数を獲得する機能を持っている。
# ❌ 複数機能を持っているため命名に困り、関数名もどんな機能を持っているのかわからない。

# numbers = [1, 2, 3, 4, 5]
def calc(numbers):
    even = []
    total = 0
    for number in numbers:
        total = total + number
        if number % 2 == 0:
            even.append(number)
    return total, even

# ⭕️ 合計を計算する機能と偶数を獲得する機能を分割した。
# ⭕️ 関数名で何をするのかが判断できる。

# numbers = [1, 2, 3, 4, 5]
def get_total(numbers):
    total = 0
    for number in numbers:
        total = total + number
    return total

def get_even(numbers)
    even = []
    for number in numbers:
        if number % 2 == 0:
            even.append(number)
    return even

---

２.不要なコードを削除していない

難易度：★☆☆　頻出度：★★★

不要となったクラスや関数・メソッドをコメントアウトして放置していたり、実行しないからといってコードを放置していませんか？

途中で不使用となったコードをは削除をしないと保守性・可読性の低下につながります。不要だと判断したコードは削除するようにしましょう。

---

３.例外処理を行なっていない

難易度：★☆☆　頻出度：★★☆

例外処理とは、プログラムがある処理を実行している途中で、なんらかの異常が発生した場合に、現在の処理を中断し、別の処理を行うことです。例外処理を記述することで、予期していなかったエラーに対応できるプログラムを書くことができます。

例えば、複数のファイルを読み込んで処理をするプログラムで「読む込むはずのあるファイルが存在しない」となったとき、１つのエラーで全体の実行が止まります。しかし、１つのエラーが出ても他のファイルに処理を施したいという場合、例外処理を組み込むことで、１つのエラーが起きても処理を続けることができます。

また、例外処理ではどんなエラーが発生しうるのかを想定しておき、try節でエラーをキャッチした後のexcept節で、エラー発生時にどのような処理をするかを決めておくことでエラーの原因の修正に役立ちます。

# ❌ 例外処理がない
# ❌ 一つでも読み込めないファイルがあると処理が止まる

for filename in csv_files:
    df = pd.read_csv(filename)

# ⭕️ 例外処理を入れた。
# ⭕️ どのファイルでエラーが起きたのかわかる。

for filename in csv_files:
    try:
        df = pd.read_csv(filename)
    except FileNotFoundError as e:
        print("{} not found.".format(filename))

---

４.マジックナンバーの使用

難易度：★☆☆　頻出度：★★☆

マジックナンバーとは何らかの識別子もしくは定数として用いられる、プログラムのソースコード中に書かれた具体的な数値のことです。プログラムを書いた本人は数値の意図を把握していても、他のプログラマまたは本人が数値の意図を忘れたときにその数値がどのようなものなのかがわからず、保守性・可読性が低下します。

# 税込み価格を計算する関数

# ❌ 税率が変化したときに1.08がなんなのか不明瞭
def calculate_tax_included_price(price):
    return price*1.08

# ⭕️　1.08をtax_rateという変数に格納
# ⭕️　1.08がなんの数値なのかがわかる
def calculate_tax_included_price(price):
    TAX_RATE = 1.08
    return price*TAX_RATE

※簡単のために、よくマジックナンバーの例に挙げられる消費税に関するコードを使用しました。
※浮動小数点演算では誤差が生まれる可能性があるため、実際のお金の計算をする際にはdemicalモジュールを使用するなどの対処が必要です。

---

５.組み込み関数に代入

難易度：★☆☆　頻出度：★☆☆

組み込み関数とは、効率的にプログラミングをするためにあらかじめ用意されている関数のことです。Pythonではint(),len(),max()などがあります。

これらのような元々組み込まれている関数と同じ名前で変数や関数を定義してしまうと上書きができてしまいます。

例えば、いくつかの要素の中から最大値を返してくれる組み込み関数max()にある値を入れたり、挙動の違う関数として再定義してしまうと、元々の組み込み関数max()として機能しなくなってしまうのです。

・通常の挙動

max(2,3,4)
# => 4

・max関数を上書きしたときの挙動

max = 10
max(2,3,4)
# => TypeError: 'int' object is not callable

これをしてしまうと、共同で開発をする際に可読性・保守性の低下にもつながります。組み込み関数かどうかを全て覚えて判断するのはなかなか大変なので、エディタに頼りましょう。

---

６.境界値でうまく動作しない

難易度：★★☆　頻出度：★★★

実装をする上でよく起こるミスとして、境界値でうまく動作しないということがあります。このようなミスが起きないように境界を見つけ、テストケースを用意しミスを防ぐことが重要です。

例）０点から１００点満点のテストの評価
・７０点以上・・・goodと出力
・３０点以上７０点未満・・・okと出力
・３０点未満・・・badと出力

# ❌　境界値での分岐をミス
score = 30
print('bad' if score < 30 else 'ok' if 30 < score and score < 70 else 'good')

# => good

# ❌　無効なデータを考慮していない
score = 101
print('bad' if score < 30 else 'ok' if 30 <= score and score < 70 else 'good')

# => good

例を見て、こんなミスはしないと思うかもしれませんが、実際にはとても分かりづらい境界値などがあったりします。「境界値でミスがあるかもしれない」と考えてミスを発見する努力が大事です。

---

７.オリジナリティのあるコード

難易度：★★☆　頻出度：★★★

自分なりの処理

初心者は実務でコードを書く際、知識不足から自分なりの処理をしたコードを書きたくなってしまうかもしれません。しかし、これでは可読性を下げてしまいます。

既存のコードで似たような処理をしているものがある場合は参考にしてコードを書くことが重要です。

書き方に揺らぎがある
書き方に揺らぎがあるとはどういうことでしょうか？

実はプログラムを書く上で可読性を上げるための基本的なルールであったり、チーム内でコードの書き方にルールを設けている場合があります。これらのルールに従っていないことを「揺らぎ」と言います。

ルールがある場合にはそのルールに従いましょう。また、ルールが存在しない場合でも、自分のコードの中で揺らぎがないようにするのが良いとされています。

簡単な書き方の揺らぎの例を紹介します。

例１）大小関係
以下の例ではどちらの記述方式を採用しても同じ挙動を示しますが、左は小さいもの、右は大きいものという原則に従い、②を使って、①を避けたりする場合があります。しかし、チーム内のルールがある場合にはそちらに従うのが良いです。

# ①
b > a
# ② 
a < b

例２）リストの作成
以下の例を見るとリストの作成と言ってもいくつかの書き方があります。同じ結果が得られても、人それぞれの書き方をしてしまうと読みにくいコードになってしまうのです。

# ① 
numbers = [0, 1, 2, 3, 4]

# ② 
numbers = [i for i in range(5)]

# ③ 
numbers = []
for i in range(5):
    numbers.append(i)

---

８.ネストが深い

難易度：★★☆　頻出度：★★★

ネストが深いと、構造が複雑になり、何をしているコードなのか分かりにくくなったり、コードが横に広がり、視覚的に見づらくなったりします。

ネストを深くしないためのコードの記述方法を紹介します。紹介する例は非常に単純なように思いますが、積み重なるとかなりネストを削減できます。

これらの例は「７.オリジナリティのあるコード」にもつながってきます。

早期リターン
簡単に言えばreturnを使うことでifなどによるネストを減らすテクニックです。
早期リターンを行うことで以下のようなメリットが得られます。

・処理が軽くなる
・行数が減って見通しが良い
（この記事の例は行数が増えていますが、基本的には減ることが多いです。）
・テストが書きやすい
・条件の追加がやりやすい

例を見ましょう。

# 🔺

def fizzbuzz(i):
    if not (type(i) is int):
        print("The 'i' is not int!!")
    else:
        if i % 3 == 0 and i % 5 == 0:
            print("FizzBuzz")
        elif i % 3 == 0:
            print("Fizz")
        elif i % 5 == 0:
            print("Buzz")
        else:
            print(i)

# ⭕️ ネストが浅くなった

def fizzbuzz(i):
    # iがintでない場合はエラーメッセージを表示し、処理を中断する
    if not (type(i) is int):
        print("The 'i' is not int!!")
        return

    # fizzbuzzの処理を行う
    if i % 3 == 0 and i % 5 == 0:
        print("FizzBuzz")
    elif i % 3 == 0:
        print("Fizz")
    elif i % 5 == 0:
        print("Buzz")
    else:
        print(i)

※今回の例では処理が大きく軽くなることはありませんが、早期リターンは余分なロジックを実行する前に関数を出ることができるのでループを使う処理などでは処理がかなり軽くなる場合があります。

内包表記を使用する
※やりすぎるとかえって可読性を下げる可能性があるので注意

#　🔺

numbers = []
for i in range(5):
    numbers.append(i)

#　⭕️ ネストなし
#　⭕️ ３行かかっていた処理が１行

numbers = [i for i in range(5)]

三項演算子を使用する
※複雑な条件だったり、やりすぎるとかえって可読性を下げる可能性があるので注意

#　🔺

#target = 2
if target % 2 == 0:
    result = target*2
else:
    result = target*3

#　⭕️ ネストなし
#　⭕️ ４行かかっていた処理が１行

#target = 2
result = target*2 if target % 2 == 0 else target*3

---

９.クラスや関数を使用していない

難易度：★★☆　頻出度：★★★

自分が書いたコードでクラスや関数・メソッド化できるものがあるかどうかを常にチェックしましょう。

• クラスを使うメリット

  • コードの重複を減らせる
  • 必要に応じた最低限のコード追加で済む
  • うっかりミスを防げる
  • 複数人で開発するときに便利

• 関数・メソッドを使うメリット

  • コードの重複を減らせる
  • コードがわかりやすくなる
  • プログラマが処理を意識しなくていい

などが挙げられます。

関数は初心者がプログラミングを勉強する際に初期に学ぶ技術ですが、クラスに関しては積極的に学ぼうとしなければあまり初心者が使う技術ではないと思いますが、実務では必須の技術になってくるので、実務に入る前に大体でも良いので理解をしておくとスムーズかもしれません。

自分は以下のサイトでクラスの概要を理解し、実務でコードを読むときに理解がしやすかったです。

【参考】Pythonの「クラス」を理解、オブジェクト指向プログラミングの基本を押さえる

---

１０.識別子の命名が雑

難易度：★★☆　頻出度：★★★

共通ルールに従っていない
関数名やクラス名、ファイル名などでは、プログラマの間で決められている名付け方がいくつかあり、識別子の用途に応じて、下の６つから選んで使います。選び方はチームやプロジェクト内でも違う場合があるので確認しましょう。

# キャメルケース・・・単語の一文字目を大文字にする表記方法
GetColumnName #アッパーキャメルケース
getColumnName #ローワーキャメルケース

# スネークケース・・・単語間をアンダースコア(_)で区切って表記する方法
get_column_name
GET_COLUMN_NAME #アッパースネークケースorコンスタントケース

# チェインケース・・・単語間をハイフン(-)で区切って表記する方法
get-column-name
Get-Column-Name #トレインケース

何もわからない識別子名
識別子名が直接プログラムの動作に影響しないからと言って、適当な命名をしていませんか？

１人で練習問題を解いたり、学校の課題をする際などには自分で後から見返したりすることがないからといって適当に変数名をaやbにしている人は少なくないのではないでしょうか。

命名を行う際、「素早くコードの意図を把握することができる命名」をする必要があります。

しかし、良い命名をするのは初心者ではかなり難しいです。まずは、「何もわからない識別子名」をつけるのをやめましょう。意識的に「素早くコードの意図を把握することができる命名」をつけるようにしましょう。

コツとしては、多少識別子名が長くなってしまっても情報の過不足をなくすことです。

# ❌ 変数名だけではどんな情報が入っているのかわからない
a = [68, 90, 73, 88, 76] #数学の点数のリスト

# ⭕️ 変数名だけでどんな情報が入っているのかわかる
# ⭕️ コメントが不要
# ⭕️ 型がわかる
math_score_list = [68, 90, 73, 88, 76] 

文法のおかしい識別子名
基本的に識別子名は文法上正しい命名をする方が好ましいです。命名をする際は文法的におかしくないかを確認しましょう。

# 🔺 createが動詞
create_user = User(name="Taro", age=30)

# ⭕️ 受け身の形
created_user = User(name="Taro", age=30)

動詞で始まっていない関数・メソッド名
ほとんどの関数やメソッドは、動詞から始まることが好ましいです。副詞句が必要な場合は、動詞より後ろ側に記述するようにしましょう。

例１）

# ❌ 動詞が入っていない
def user_creation():
  pass

# ⭕️ 動詞を入れた
def create_user():
  pass

例２）

# 🔺 動詞で始まっていない
def securely_send_message():
  pass

# ⭕️ 動詞で始まっている
def send_message_securely():
  pass

【参考】名前付け重要。または、良いコードは良い名前から生まれるんです。
【参考】識別子（変数名や関数名など）の命名ガイドライン

---

１１.実行時間・計算量を考えない

難易度：★★☆　頻出度：★★☆

重い処理や計算量を意識しない実装はかなりの損を生んでしまいます。方法によって計算時間に大きな差が出る例として、ソートを見ましょう。

ソート方法	平均計算時間	n = 10,000	n = 20,000	n = 30,000
挿入ソート	$O(n^2)$	6.73秒	27.22秒	61.25秒
バブルソート	$O(n^2)$	15.08秒	60.50秒	130.46秒
標準ライブラリ	$O(n\log{n})$	0.002秒	0.004秒	0.007秒

上の表を見ると圧倒的に標準ライブラリが速いことがわかります。

このようにどの処理の方法を取るかによって実行時間に大きな差が出てきてしまう場合があったりします。

上記では標準ライブラリを使うことで遅い処理を回避できますが、上記のようなもの以外に初心者がやってしまう遅い処理が存在します。

以下では自分の行ってしまった遅い処理について紹介します。

例１）Pythonのリストにおけるある値vの検索
これはかなり多くの人が以下の例のように書いてしまっているのではないかと思います。そこまで大きなリストではない場合特に問題にはならないのですが探索範囲が大きくなるとリストにおけるある値vの検索は非常に時間がかかってしまいます。しかし、set()を使用すると計算時間が大きく削減できます。

この理由は「list()はリスト構造」で「set()はハッシュテーブル」であることです。リスト構造では全探索をするのに対し、ハッシュ値を元に探索をすることができるので計算量が少なくなります。

# 🔺
if v in hoge_list:

# ⭕️ 計算時間が大体１００分の１
if v in hoge_set:

詳しい理由を知りたい方は下記サイトを参考にしてみてください。

【参考】
Pythonで"in list"から"in set"に変えただけで爆速になった件とその理由

例２）DataFrameの扱い
実務では、機械学習の案件などで大量のデータを扱う際、pandasを使って処理することがあります。DataFrameをfor文で扱うと非常に処理が遅くなってしまうので、以下のサイトを見てこちらの解決策を知っておきましょう

【参考】
お前らのpandasの使い方は間違っている

二つの例を挙げましたが、知らずのうちに行ってしまっている遅い処理があるかも、、と不安な方は下記サイトなどで勉強しましょう。

Pythonプログラムが遅い！高速化したい！そんな時は...

Pythonが遅いと感じたら！見直すべき高速化のポイント

---

１２.適切でないコメント

難易度：★★☆　頻出度：★★☆

美しいコードの例として、コメントがなくても理解できるコードということがよく言われます。

これはごもっともなのですが、実際はそんなコードを書くことができるのはとても経験豊富なエンジニアだけです。

では、初心者や実務に入ったばかりの人はどうするのが良いのでしょうか？

結論は「適切なコメントを残すようにする」です。以下の必要なコメント・必要のないコメントを参考にどのようなコメントを残すのが良いのかを理解しておきましょう。

必要のないコメント
・コードを見ればわかるコメント
・ひどいコードを補うためのコメント
補助的なコメントではなくコメントを書くべきではなく、コードを修正しましょう。

必要なコメント
・急いで開発する際に手が及ばなかった・修正するべき点などをわかるようにするためのコメント
・普通の書き方に従わない場合、なぜそう書いたのかの理由
・参考にしたページのURL
・うまくいかなかった方法

必要なコメントの例を紹介します。
・急いで開発する際に手が及ばなかった・修正するべき点などをわかるようにするためのコメント
このようなコメントを残しておくことで時間ができたときに修正するべきポイントがわかったり、コードを見た他の人が修正してくれる場合もあります。問題を見える状態にしておくことが重要です。

// この処理は無理やりなので修正

// 高速なアルゴリズムに変更が必要

・普通の書き方に従わない場合、なぜそう書いたのかの理由
「何をしているか」はコードからわかりますが「なぜそうしているか」はコードからはわからないので理由を書いておくことは重要です。

// xxxライブラリのバグにより期待するxxxの値が返ってこないためxxxする

・参考にしたページのURL
コメントをたくさん残すよりもわかりやすくまとめてある記事などを見てもらった方が理解がしやすいです。

// xxxの挙動については http://stackoverflow.com/questions/xxx を参照のこと

・うまくいかなかった方法
あとで誰かがリファクタリングするときに「もっといい書き方がある」と思って、もうすでに試した失敗する方法へ書き直して時間をロスするのを防ぐことができます。

// xxxという方法を取ったところ「Error: xxx xxx」というエラーが出た

例のようなコメントを残すことは非常に重要です。ただ、実務に入ってすぐは、急いで開発するということはなかなかないと思うのでリファクタリングをしたり、メンターに相談するなどして可読性の向上に努めましょう。

【参考】
【プログラミング初心者】コメントを書くとき知っておきたいこと
コードを読む人の助けになるコメントを書くために

---

１３.ミュータブルなのかイミュータブルなのかを考えていない

難易度：★★☆　頻出度：★☆☆

まずミュータブル・イミュータブルとは何なのでしょうか。

ざっくりと

・ミュータブル・・・変更可
（例 list、dict、set、bytearrayなど）
・イミュータブル・・・変更不可
（例 bool、int、flaot、complex、str、tuple、rangeなど）

と言えます。

だから何？という感じかもしれませんが、どのような問題が発生しうるのか見ていきましょう。

発生しうる問題と解決策

「num_list1はそのままにして、要素をひとつだけ変更して新しいリストnum_list2をつくりたい」という場合があったとします。

問題が起きてしまっている例

# ❌　num_list1にまで変更が及んだ
num_list1 = [1,2,3,4,5]
num_list2 = num_list1
num_list2.append(3)

print(num_list1)
# => [1, 2, 3, 4, 5, 3]

改善例

# ⭕️ num_list1には変更が及んでいない
import copy

num_list1 = [1,2,3,4,5]
num_list2 = copy.copy(num_list1)
num_list2.append(3)

print(num_list1)
# => [1, 2, 3, 4, 5]

ではなぜ、上記の問題が起きている例では想定の結果を得られなかったのでしょうか。この理由がミュータブルなのかイミュータブルなのかを考えていないことにあります。

ミュータブルなオブジェクトでは、代入先のオブジェクトに変更を加えると、代入元のオブジェクトにまで変更が及んでしまいます。（代入元のオブジェクトに変更を加えても代入先のオブジェクトにまで変更が及ぶ）

その解決策としてcopyモジュールがあります。copyモジュールのcopy()関数を使うと、変更を加えたいオブジェクトのみを変更することができます。

どんな問題が発生してしまうのかが分かったところで、ミュータブル・イミュータブルの詳しい違いを見ていきます。

ミュータブルなオブジェクト
ミュータブルなオブジェクトでは変更可と説明しました。この場合オブジェクトに変更を加えても参照するidは変わりません。

list型を例に見ていきます。

num_list1 = [1,2,3,4,5]
num_list2 = num_list1
print('num_list1のid：{}'.format(id(num_list1)))
print('num_list2のid：{}'.format(id(num_list2)))

num_list2.append(3)     
print('num_list1のid：{}'.format(id(num_list1)))
print('num_list2のid：{}'.format(id(num_list2)))

# 変更前
# => num_list1のid ： 4314551104
# => num_list2のid ： 4314551104   # num_list1と同じid
# 変更後
# => num_list1のid ： 4314551104
# => num_list2のid ： 4314551104   # 変更を加えてもnum_list1と同じid

イミュータブルなオブジェクト
イミュータブルなオブジェクトは変更不可と説明しました。しかし、一度生成したオブジェクトに変更を加えようとするとどうなるのでしょうか。そのような場合は、別のオブジェクトが自動的に新規で生成され、変数名は、新しいオブジェクトを指すように、紐付けが変更されるのです。

str型を例に見ていきます。

string1 = 'abc'
string2 = string1
print('string1のid：{}'.format(id(string1)))
print('string2のid：{}'.format(id(string2)))

string2 = string2 + 'abc'
print('string1のid：{}'.format(id(string1)))
print('string2のid：{}'.format(id(string2)))

# 変更前
# => string1のid : 4314167344     
# => string2のid : 4314167344   # この時点ではstring１と同じ
# 変更後
# => string1のid : 4314167344
# => string2のid : 4314936624   # 変更を加えるとidが変わる

二次元配列になると、、、

二次元配列になるとdeepcopy()関数というものが必要になります。

詳しいことを解説しようとすると長くなってしまうので、詳しくはPythonのcopyとdeepcopyについてを参考にしてみてください。

---

１４.整合性の取れていない並行処理・並列処理

難易度：★★★　頻出度：★☆☆

通常Pythonでは１つずつ処理のが実行される逐次実行が行われています。プログラムの処理性能を上げるための方法として並行処理や並列処理があり、これらを行うことで処理時間の短縮が期待できます。

しかし、データの整合性、処理順などに気を付けて実装をしないと、期待した結果が得られない場合があります。並行処理や並列処理を扱った際には、ミスが起きていないかを確認しましょう。

この記事では、Pythonの並行処理を理解したい [マルチスレッド編]を引用させていただき、並行処理のミスが起きてしまっている例を見ます。

スレッドセーフとロック
並行処理についてまわる問題としてスレッドセーフがあります。

マルチスレッドで処理を並行実行する際に、共通した値をそれぞれのスレッドが読み書きするなどスレッドセーフではない処理を挟んでしまうとバグの原因になります。

スレッドセーフではない処理の例とその対処（ロック）について紹介します。

スレッドセーフではない処理
以下は、Counterというクラスのインスタンス変数をカウントアップする処理です。スレッドセーフではないので、マルチスレッドで処理すると想定通りの値にならず実行ごとに計算結果が変わってしまいます。

thread_safe.py

from concurrent.futures import (ThreadPoolExecutor, wait)


class Counter:
   def __init__(self):
       self.count = 0

   def count_up_to_1000000(self):
       for _ in range(1_000_000):
           self.count += 1

def worker(counter):
   counter.count_up_to_1000000()


def main():
   counter = Counter()
   with ThreadPoolExecutor() as executor:
       features = [executor.submit(worker, counter) for _ in range(3)]
   print(counter.count)


if __name__ == '__main__':
   main()

# ❌　3000000にならない
$ python3 thread_safe.py
1848519

$ python3 thread_safe.py
1802127

$ python3 thread_safe.py
1702320

これはself.countをインクリメントする際の値読み取り中にスレッドが切り替わり、複数スレッドで同じ値に同時にアクセスすることで不整合な状態となってしまうためです。

改善（ロックの導入）
これを改善するためには、インクリメント中にスレッドの排他制御（ロック）をする必要があります。

改善コードはこちらです。出力も想定の 3000000 になっています。

thread_safe.py

import threading
from concurrent.futures import (ThreadPoolExecutor, wait)


class Counter:
   lock = threading.Lock()

   def __init__(self):
       self.count = 0

   def count_up_to_1000000(self):
       for _ in range(1_000_000):
           with self.lock:
               self.count += 1


def worker(counter):
   counter.count_up_to_1000000()


def main():
   counter = Counter()
   with ThreadPoolExecutor() as executor:
       features = [executor.submit(worker, counter) for _ in range(3)]
   print(counter.count)


if __name__ == '__main__':
   main()

$ python3 thread_sage.py
3000000

Counter クラスの中でLockインスタンスを作り、カウントアップをLockインスタンスのコンテキストマネージャ内で実行しています。

with self.lock:のブロック内で行われる処理はロックがかかるので、カウントアップ時の複数スレッドでの値共有という不具合の原因が解消されます。

【参考・引用】
並列処理
[Python] スレッドで実装する
並行処理
Pythonの並行処理を理解したい [マルチスレッド編]

まとめ

コードレビューで指摘されたことをいくつか紹介してきました。

もちろんミスはない方が良いですが、実務に入ってすぐからこれらのことを全て完璧にする必要はありません。紹介してきたことを意識しておくだけでもだいぶ良いコードが書けるようになると思います。

実務経験をすることでモチベーションが上がりますし、何よりとても勉強になることが多いので、恐れず実務経験が積める場所に参加してみてはいかがでしょうか。

おわりに

弊社では、経験の有無を問わず、社員やインターン生の採用を行っています。

興味のある方はこちらをご覧ください。