【コードレビュー】知り合いが書いたプログラムをレビューしてみた3

2018年11月21日
BG
コードレビュー

とある知り合いがBGで「バブルソート」を書いたのでレビューしてみた。

レビュー

下記は知り合いが書いたプログラム。今回も程よく突っ込みどころがある。

//※変更時　配列数
memory[ auto ] name ARRAY_COUNT = 7

function show_result()
    memory[ auto ] name count = 0
    memory[ auto ] name max_count = ARRAY_COUNT - 1
    show( "バブルソート完了" )
    show( "配列数：" . ARRAY_COUNT )
    
    while count <= max_count
        show( "memory" . "[" . count . "]：" . memory[ count ] )        
        count = count + 1

function decide_big_numbers_a_to_b( memory[ auto ] name array_num_a, memory[ auto ] name array_num_b )
    memory[ auto ] name temp_a = 0
    memory[ auto ] name temp_b = 0
    
    if memory[ array_num_a ] > memory[ array_num_b ]
        temp_a = memory[ array_num_a ]
        temp_b = memory[ array_num_b ]
        memory[ array_num_a ] = temp_b
        memory[ array_num_b ] = temp_a
        
function main()
    memory[ auto ] name compare_count = 0
    memory[ auto ] name compare_max_count = ARRAY_COUNT - 1
    memory[ auto ] name compare_array_num_a = 0
    memory[ auto ] name compare_array_num_b = 1
    memory[ auto ] name is_finish = false
    memory[ auto ] name is_sort_end = false
    
    //※変更時 配列追加
    memory[ 0 ] = 40
    memory[ 1 ] = 25
    memory[ 2 ] = 30
    memory[ 3 ] = 10
    memory[ 4 ] = 20
    memory[ 5 ] = 15
    memory[ 6 ] = 35
    
    while is_finish == false
        while is_sort_end == false
            //AとBを比較して、大きい数字を決定
            decide_big_numbers_a_to_b( compare_array_num_a, compare_array_num_b )
            
            //比較回数と比較配列要素数の加算
            compare_count = compare_count + 1
            compare_array_num_a = compare_array_num_a + 1
            compare_array_num_b = compare_array_num_b + 1
            
            //比較回数が最大比較回数に達した場合
            if compare_count == compare_max_count
                is_sort_end = true
        
        //最後の配列値は確定したので、最大比較数を減算
        compare_max_count = compare_max_count - 1
        
        //初期化
        compare_count = 0
        compare_array_num_a = 0
        compare_array_num_b = 1
        is_sort_end = false
        
        if compare_max_count == 0
            is_finish = true
    show_result()

気になる点

該当行理由修正例
6と10
29と41
30と42		変数を作成する必要がない。変数を削除し、条件を変更する。
while count < ARRAY_COUNT
while compare_max_count > 0
while compare_count != compare_max_count
14関数名が処理の内容と一致しない。適切な関数名に変更する。
swap_if_larger( memory[ auto ] name array_num_a, memory[ auto ] name array_num_b )
15と19
16と20		変数を事前に作成しておく必要がない。代入する時に作成する。
memory[ auto ] name temp_a = memory[ array_num_a ]
memory[ auto ] name temp_b = memory[ array_num_b ]
18分岐によりネストが深くなっている。条件を逆にし、return文で処理を中断する。( 早期リターンする。 )
if memory[ array_num_a ] <= memory[ array_num_b ]
25～28変数名が冗長。変数名のcompare_を除く。
( 付けるにしてもcompareは動詞なので、comparingの方がよい。 )
memory[ auto ] name count = 0
memory[ auto ] name max_count = ARRAY_COUNT - 1

修正した場合

上記の気になる点を全て修正すると、下記のようなプログラムとなる。

memory[ auto ] name ARRAY_COUNT = 7

function show_result()
    memory[ auto ] name count = 0
    show( "バブルソート完了" )
    show( "配列数：" . ARRAY_COUNT )
    
    while count < ARRAY_COUNT
        show( "memory" . "[" . count . "]：" . memory[ count ] )        
        count = count + 1

function swap_if_larger( memory[ auto ] name array_num_a, memory[ auto ] name array_num_b )
    if memory[ array_num_a ] <= memory[ array_num_b ]
        return
    
    memory[ auto ] name temp_a = memory[ array_num_a ]
    memory[ auto ] name temp_b = memory[ array_num_b ]
    memory[ array_num_a ]      = temp_b
    memory[ array_num_b ]      = temp_a
    
function main()
    memory[ auto ] name count = 0
    memory[ auto ] name max_count = ARRAY_COUNT - 1
    memory[ auto ] name array_num_a = 0
    memory[ auto ] name array_num_b = 1
    
    memory[ 0 ] = 40
    memory[ 1 ] = 25
    memory[ 2 ] = 30
    memory[ 3 ] = 10
    memory[ 4 ] = 20
    memory[ 5 ] = 15
    memory[ 6 ] = 35
    
    while max_count > 0
        while count != max_count
            swap_if_larger( array_num_a, array_num_b )
            
            count       = count       + 1
            array_num_a = array_num_a + 1
            array_num_b = array_num_b + 1
        
        max_count   = max_count - 1
        count       = 0
        array_num_a = 0
        array_num_b = 1
        
    show_result()

不要な変数が多かったため、それらがなくなりスッキリとしたプログラムになった。

理咲のプログラム

ちなみに自分がバブルソートを書くと下記のプログラムになる。

memory[ auto ] name COUNT = 10

function main()
    set_default_values()
    show_default_values()
    
    sort_in_ascending_order()
    show_sorted_values()
    
function set_default_values()
    memory[ 0 ] = 7
    memory[ 1 ] = 2
    memory[ 2 ] = 1
    memory[ 3 ] = 5
    memory[ 4 ] = 4
    memory[ 5 ] = 9
    memory[ 6 ] = 6
    memory[ 7 ] = 3
    memory[ 8 ] = 8
    memory[ 9 ] = 0

function show_default_values()
    show( "【 ソート前 】" )
    show_values()

function show_sorted_values()
    show( "【 ソート後 】" )
    show_values()
    
function show_values()
    memory[ auto ] name i = 0
    
    while i < COUNT
        show( "memory[ " . i . " ] = " . memory[ i ] )
        i = i + 1

function sort_in_ascending_order()
    memory[ auto ] name i = 0
    memory[ auto ] name j = 0
    
    while i < COUNT
        j = i + 1
        
        while j < COUNT
            swap_if_larger( i, j )
            j = j + 1
        
        i = i + 1

function swap_if_larger( memory[ auto ] name i, memory[ auto ] name j )
    if memory[ i ] <= memory[ j ]
        return
    
    memory[ auto ] name temp = memory[ i ]
    memory[ i ]              = memory[ j ]
    memory[ j ]              = temp

知り合いとの主な違いは、ソート時に値を前方から確定させているところ( 知り合いは後方から )。この違いがあったため、初見では若干戸惑った。