株式会社ゆめみさんの採用コーディング試験模試のコードレビューをお願い致します。

Question

https://www.yumemi.co.jp/serverside_recruit
上記はゆめみさんの採用コーディング試験の模試をpythonで実装しました。

以下コードです。

import csv
import collections

source_file = open('output.txt', 'w')

def get_total_score(csv_reader):

    player_to_ttlscore = {}
    for row in csv_reader:
        
        if row[1] not in player_to_ttlscore.keys():
            player_to_ttlscore[row[1]] = {}
            player_to_ttlscore[row[1]]['cnt'] = 1
            player_to_ttlscore[row[1]]['total'] = int(row[2])

        else:
            player_to_ttlscore[row[1]]['cnt'] += 1
            player_to_ttlscore[row[1]]['total'] += int(row[2])
        
    return player_to_ttlscore

def get_avg_score(player_to_ttlscore):

    avgscore_to_players = {}

    for player in player_to_ttlscore:
        avg_score = int(player_to_ttlscore[player]['total'] / player_to_ttlscore[player]['cnt'])

        if avg_score not in avgscore_to_players.keys():
            avgscore_to_players[avg_score] = [player]

        else:
            avgscore_to_players[avg_score].append(player)

    ## 平均値の降順にする
    return collections.OrderedDict(sorted(avgscore_to_players.items(), reverse=True))

def output_ranking(ordered_ranking):
    rank = 1
    for avg_score, player_with_samescore in ordered_ranking.items():

        # ランキング10で終了
        if rank > 10:
            break

        ## 同じスコアの人がいないとき
        if len(player_with_samescore) == 1:
            print("{},{},{}".format(rank, str(player_with_samescore)[2:-2], avg_score), file=source_file)
            rank += 1

        else:
            for j in range(len(player_with_samescore)):
                print("{},{},{}".format(rank, player_with_samescore[j], avg_score), file=source_file)

            ## 同スコアの数だけ人数分ランクの数を増やす
            rank += len(player_with_samescore)

with open('dummy1.txt') as csv_file:
    csv_file.readline()
    csv_reader = csv.reader(csv_file, delimiter=',')

    player_to_ttlscore = get_total_score(csv_reader)

    ordered_ranking = get_avg_score(player_to_ttlscore)

    output_ranking(ordered_ranking)

ダミーデータです

create_timestamp,player_id,score
2021/01/02 13:00,player0002,10000
2021/01/06 13:00,player0002,5000
2021/01/06 13:00,player0003,2000
2021/01/06 13:00,player0003,5000
2021/01/06 13:00,player0005,5300
2021/06/19 22:06,player3430,6000

https://qiita.com/taruhachi/items/0318c1a603478058d863
こちらの評価基準のほうに

通常プログラムはそのプログラム単体では利用されず、入力されるデータも他のプログラムにより生成されることが多いが、そのデータが急に変更されたときなどに破壊的な問題を起こさないか

とあるのですが、私のコードはこれを満たしていますでしょうか？

また、こちらの上記のコードに例外処理をいれるとすれば、どのようにすればよいでしょうか？

オブジェクト指向での実装をこれから行うつもりなのですが、
コーディング試験の採点側からすると、OOPでの実装はやはりプラスの評価なのでしょうか？

どれか一つで構いませんのでアドバイスいただけますと幸いです。
よろしくお願いいたします

Comments

[javahack]

正確なところは出題者にしかわかりません。
出題者に聞きましょう。

[68user]

書いてあるとおり、何を重視したのかアピールコメントに書きましょう。個人的にはコードの良し悪しよりコメントの方が10倍大事と思います。
破壊的云々とあるので、カラム数・日時・数値チェックを入れればよいのではというのもありますが、ランキングという性質からすると異常時に終了すべきか異常データとしてスキップすべきかは要検討でしょう。あと何百万件などと大量データになるかもしれないスコア情報集計をメモリに溜め込むのはどうかなと思いますが「可読性重視で1行ずつオブジェクトに格納しました」と書いてあるならまあよしかなと。

[68user]

失礼。全行メモリに溜め込んでるかと勘違いしておりましたがそうではありませんでした。

[sdfguu6u]

回答ありがとうございます。
＞破壊的云々とあるので、カラム数・日時・数値チェックを入れればよいのではというのもありますが、ランキングという性質からすると異常時に終了すべきか異常データとしてスキップすべきかは要検討でしょう。

こちらの意味が私にはうまく理解で来ませんでした。
異常データとしてスキップとは具体的にどういう意味なのでしょうか。
例外として処理するということなのでしょうか。

お手数おかけしますがよろしくお願いいたします

[68user]

端的にいえば異常データが見つかったら即終了するのか、あるいはスキップして処理を続行するのかということです。正解はないのでなぜそうしたかを書けばよいと思います。

[sdfguu6u]

なるほど参考になりました。
意図をかくことが大切なのですね。
ありがとうございました。

Answer 1

私のコードはこれを満たしていますでしょうか？

満たしているかどうかはゆめみさんが評価することですが、少なくとも

source_file = open('output.txt', 'w')

と記述した時点で output.txt が存在する場合は内容を消してしまうので、ファイルを読み込む前にシステムにとっては破壊的な変更をしていることになります。
また、source という単語は入力（データの元）を指すことが多いです。出力先に source という単語を使うとかなり混乱します。

あとせめて入出力のルールくらいはちゃんと満たしましょう。
実行時にコマンドライン引数でファイル名を指定しているところとか、標準出力に結果を出力しているところとか、ヘッダー行がないところとか。

などなど、いろいろとコメントは可能ではあるのですが、できれば自分で考えてみてください。

Comments

[mather]

なお、これ以上のコードレビューはお断りいたしますし、「上記のコードに例外処理をいれるとすれば、どのようにすればよいでしょうか？」は質問ではなくコード作成依頼なので論外です。自分で書いてみてから質問してください。

[sdfguu6u]

レビューいただきありがとうございました。