プログラムの拡張性を高めるための改善点、何が良くなかったのでしょうか

Question

独学のずぶの素人です。
この問題の結果だけ求めるプログラムを書いたのですが、後になって結果がどの数値から求められたのか
気になり改良しようとしたのですが面倒になりやめました
最初からどのように書いていれば拡張性が高いプログラムを作ることが出来たのでしょうか？

'''
「格子内の最大の積」

08 02 22 97 38 15 00 40 00 75 04 05 07 78 52 12 50 77 91 08
49 49 99 40 17 81 18 57 60 87 17 40 98 43 69 48 04 56 62 00
81 49 31 73 55 79 14 29 93 71 40 67 53 88 30 03 49 13 36 65
52 70 95 23 04 60 11 42 69 24 68 56 01 32 56 71 37 02 36 91
22 31 16 71 51 67 63 89 41 92 36 54 22 40 40 28 66 33 13 80
24 47 32 60 99 03 45 02 44 75 33 53 78 36 84 20 35 17 12 50
32 98 81 28 64 23 67 10 26 38 40 67 59 54 70 66 18 38 64 70
67 26 20 68 02 62 12 20 95 63 94 39 63 08 40 91 66 49 94 21
24 55 58 05 66 73 99 26 97 17 78 78 96 83 14 88 34 89 63 72
21 36 23 09 75 00 76 44 20 45 35 14 00 61 33 97 34 31 33 95
78 17 53 28 22 75 31 67 15 94 03 80 04 62 16 14 09 53 56 92
16 39 05 42 96 35 31 47 55 58 88 24 00 17 54 24 36 29 85 57
86 56 00 48 35 71 89 07 05 44 44 37 44 60 21 58 51 54 17 58
19 80 81 68 05 94 47 69 28 73 92 13 86 52 17 77 04 89 55 40
04 52 08 83 97 35 99 16 07 97 57 32 16 26 26 79 33 27 98 66
88 36 68 87 57 62 20 72 03 46 33 67 46 55 12 32 63 93 53 69
04 42 16 73 38 25 39 11 24 94 72 18 08 46 29 32 40 62 76 36
20 69 36 41 72 30 23 88 34 62 99 69 82 67 59 85 74 04 36 16
20 73 35 29 78 31 90 01 74 31 49 71 48 86 81 16 23 57 05 54
01 70 54 71 83 51 54 69 16 92 33 48 61 43 52 01 89 19 67 48

上の 20×20 の格子のうち, 上下左右斜めのいずれかの方向で連続する4つの数字の積のうち最大のものはいくつか?
'''

import numpy as np

def right( a, row, column ):
    tmp = 1
    for i in range( 4 ):
        tmp *= a[ row ][ column + i ]
    return tmp
    
def under( a, row, column ):
    tmp = 1
    for i in range( 4 ):
        tmp *= a[ row + i ][ column ]
    return tmp
    
def bottom_right( a, row, column ):
    tmp = 1
    for i in range( 4 ):
        tmp *= a[ row + i ][ column + i ]
    return tmp
    
def upper_right( a, row, column ):
    tmp = 1
    for i in range( 4 ):
        tmp *= a[ row - i ][ column + i ]
    return tmp
    
standard_value = 20

a = np.loadtxt('11.txt', delimiter = ' ')
tmp = 0

for row in range( standard_value ):
    for column in range( standard_value ):
        if standard_value-4 >=  column:
            tmp = right( a, row, column ) if tmp < right( a, row, column ) else tmp
            
            if standard_value-4 >=  row:
                tmp = bottom_right( a, row, column ) if tmp < bottom_right( a, row, column ) else tmp
            elif row >= 3:
                tmp = upper_right( a, row, column ) if tmp < upper_right( a, row, column ) else tmp
        if standard_value-4 >=  row:
            tmp = under( a, row, column ) if tmp < under( a, row, column ) else tmp
print( int(tmp) )

#51267216

Comments

[coco_bauer]

解こうとする問題の内容を、質問の中に正しく簡潔に記載してください。
・「上の 20×20 の格子のうち, 斜めに並んだ4つの数字が赤くマークされている」と書かれていますが、『赤くマークされた４つの数字』は何を意味するのですか？　何かに使うのですか？

・「上の 20×20 の格子のうち, 上下左右斜めのいずれかの方向で連続する4つの数字の積のうち最大のものはいくつか?」
と書かれていますが、”上下左右斜めのいずれかの方向で連続する”というのは、隣り合う数字が「上下左右斜めのいずれかの位置にあれば良い」という意味ですか、それとも「４つの数字は、上下左右斜めのいずれかの方向に直線的に並んでいる」という意味ですか？

[malia]

失礼致しました、修正致しました。
・赤マーク(それらの数字の積は 26 × 63 × 78 × 14 = 1788696 となる.)というのは環境の違いで反映できなかった箇所です、削除しました。

・上下左右斜めのいずれかの方向に直線的に並んでいるという意味です。

[t_obara]

どのような点で面倒に感じたのでしょうか。その面倒くささがヒントになるかと思います。

Answer 1

シンプルに書くのが一番かなと思います。
最初は

values = [
[ 8,  2, 22, 97, 38, 15,  0, 40,  0, 75,  4,  5,  7, 78, 52, 12, 50, 77, 91,  8],
[49, 49, 99, 40, 17, 81, 18, 57, 60, 87, 17, 40, 98, 43, 69, 48,  4, 56, 62,  0],
[81, 49, 31, 73, 55, 79, 14, 29, 93, 71, 40, 67, 53, 88, 30,  3, 49, 13, 36, 65],
[52, 70, 95, 23,  4, 60, 11, 42, 69, 24, 68, 56,  1, 32, 56, 71, 37,  2, 36, 91],
[22, 31, 16, 71, 51, 67, 63, 89, 41, 92, 36, 54, 22, 40, 40, 28, 66, 33, 13, 80],
[24, 47, 32, 60, 99,  3, 45,  2, 44, 75, 33, 53, 78, 36, 84, 20, 35, 17, 12, 50],
[32, 98, 81, 28, 64, 23, 67, 10, 26, 38, 40, 67, 59, 54, 70, 66, 18, 38, 64, 70],
[67, 26, 20, 68,  2, 62, 12, 20, 95, 63, 94, 39, 63,  8, 40, 91, 66, 49, 94, 21],
[24, 55, 58,  5, 66, 73, 99, 26, 97, 17, 78, 78, 96, 83, 14, 88, 34, 89, 63, 72],
[21, 36, 23,  9, 75,  0, 76, 44, 20, 45, 35, 14,  0, 61, 33, 97, 34, 31, 33, 95],
[78, 17, 53, 28, 22, 75, 31, 67, 15, 94,  3, 80,  4, 62, 16, 14,  9, 53, 56, 92],
[16, 39,  5, 42, 96, 35, 31, 47, 55, 58, 88, 24,  0, 17, 54, 24, 36, 29, 85, 57],
[86, 56,  0, 48, 35, 71, 89,  7,  5, 44, 44, 37, 44, 60, 21, 58, 51, 54, 17, 58],
[19, 80, 81, 68,  5, 94, 47, 69, 28, 73, 92, 13, 86, 52, 17, 77,  4, 89, 55, 40],
[ 4, 52,  8, 83, 97, 35, 99, 16,  7, 97, 57, 32, 16, 26, 26, 79, 33, 27, 98, 66],
[88, 36, 68, 87, 57, 62, 20, 72,  3, 46, 33, 67, 46, 55, 12, 32, 63, 93, 53, 69],
[ 4, 42, 16, 73, 38, 25, 39, 11, 24, 94, 72, 18,  8, 46, 29, 32, 40, 62, 76, 36],
[20, 69, 36, 41, 72, 30, 23, 88, 34, 62, 99, 69, 82, 67, 59, 85, 74,  4, 36, 16],
[20, 73, 35, 29, 78, 31, 90,  1, 74, 31, 49, 71, 48, 86, 81, 16, 23, 57,  5, 54],
[ 1, 70, 54, 71, 83, 51, 54, 69, 16, 92, 33, 48, 61, 43, 52,  1, 89, 19, 67, 48],
]

standard_value = 20
NUMBERS=4
a = 0

for base_y in range(standard_value):
    for base_x in range(standard_value):
        tmp = [ 1, 1, 1, 1 ]
        for i in range(NUMBERS):
            if (base_x+NUMBERS <= standard_value):
                tmp[0] *= values[base_y][base_x+i]
            else:
                tmp[0] = 0

            if (base_y+NUMBERS <= standard_value):
                tmp[1] *= values[base_y+i][base_x]
            else:
                tmp[1] = 0

            if (base_y+NUMBERS <= standard_value) and (base_x+NUMBERS <= standard_value):
                tmp[2] *= values[base_y+i][base_x+i]
                tmp[3] *= values[base_y+i][base_x+NUMBERS-(i+1)]
            else:
                tmp[2] = 0
                tmp[3] = 0

        a = max(a, max(tmp))

print(a)

と書いて数値もみたいなら

values = [
# 略
]

standard_value = 20
NUMBERS=4
a = 0

max_x = max_y = max_dir = -1

for base_y in range(standard_value):
    for base_x in range(standard_value):
        tmp = [ 1, 1, 1, 1 ]
        for i in range(NUMBERS):
            if (base_x+NUMBERS <= standard_value):
                tmp[0] *= values[base_y][base_x+i]
            else:
                tmp[0] = 0

            if (base_y+NUMBERS <= standard_value):
                tmp[1] *= values[base_y+i][base_x]
            else:
                tmp[1] = 0

            if (base_y+NUMBERS <= standard_value) and (base_x+NUMBERS <= standard_value):
                tmp[2] *= values[base_y+i][base_x+i]
                tmp[3] *= values[base_y+i][base_x+NUMBERS-(i+1)]
            else:
                tmp[2] = 0
                tmp[3] = 0

        tmp_a = max(tmp)
        if a < tmp_a:
            max_x = base_x
            max_y = base_y
            max_dir = tmp.index(tmp_a)
            a = tmp_a

if max_dir == 0:
    print([values[max_y][max_x+i] for i in range(NUMBERS)])
elif max_dir == 1:
    print([values[max_y+i][max_x] for i in range(NUMBERS)])
elif max_dir == 2:
    print([values[max_y+i][max_x+i] for i in range(NUMBERS)])
elif max_dir == 3:
    print([values[max_y+i][max_x+(NUMBERS-(i+1))] for i in range(NUMBERS)])

print(a)

Comments

[malia]

すごいです

Answer 2

right, under, bottom_right, upper_right がほとんど同じ処理をしているので、以下のように追加引数dirでどの方向に計算するのかを渡すようにしてまとめるのはどうでしょうか。
4つの数字の積と一緒に、4つの数字と方向、開始地点も返すようにしています。

import numpy as np

from functools import reduce
from operator import mul

def calc_product(a, row, column, dir):
    dx, dy = {'right': (1, 0), 'under': (0, 1), 'bottom_right': (1, 1), 'upper_right': (1, -1)}[dir]
    numbers = [a[row + dy * i][column + dx * i] for i in range(4)]
    return reduce(mul, numbers), numbers, dir, f'row={row}, col={column}'

standard_value = 20

a = np.loadtxt('11.txt', delimiter = ' ', dtype=int)
answer = []

for row in range( standard_value ):
    for column in range( standard_value ):
        if standard_value-4 >= column:
            answer.append(calc_product(a, row, column, 'right'))
            if standard_value-4 >= row:
                answer.append(calc_product(a, row, column, 'bottom_right'))
            elif row >= 3:
                answer.append(calc_product(a, row, column, 'upper_right'))
        if standard_value-4 >=  row:
            answer.append(calc_product(a, row, column, 'under'))

print(sorted(answer)[-1])

結果が最大となるデータがどこから得られたものなのかについては、20x20の格子なので手抜きをして全てのデータを保持しておいてソートして答えを出すようにしました。

なお、元のソースにはバグがあります。
答えが最大になるのは、row=15, col=3から右上方向の 87 * 97 * 94 * 89 = 70600674 だと思います。

Answer 3

そこまでおかしな書き方には見えないですが、強いていえばもう少し適切な名前があるかと思います。
色々な考え方があるのでベストとは言わないですが、少なくとも今よりベターかなと思うものをいくつか

・standard_value -> grid_size など(さらにwidthとheightに分けた方が、正方形以外の問題も対応できる)
・right/under などの関数名を calc_right_dir などもう少し関数名で説明する
・最後に出力する tmp という変数名は max_value など（関数内の tmp はスコープが小さいのでOK）

その他、色々な意見や書き方があるかと思います。
変数名の付け方や読みやすいコードなどは下記の書籍が参考になります。
リーダブルコード

ただ、コードを書く力と同じくらい、コードを読む力も大事になります。
これくらいの量であれば諦めずにスラスラ読めるようになった方が良いです！

（一応、最低限のコードの一例を掲載します）

a = np.loadtxt('11.txt', delimiter = ' ')
tmp = 0

direction = ""
key_row = 0
key_col = 0

for row in range( standard_value ):
    for column in range( standard_value ):
        if standard_value-4 >=  column:
            right_value = right( a, row, column )
            if tmp < right_value:
                tmp = right_value
                direction = "right"
                key_row = row
                key_col = column

            if standard_value-4 >=  row:
                bottom_right_value = bottom_right( a, row, column )
                if tmp < bottom_right_value:
                    tmp = bottom_right_value
                    direction = "bottom_right"
                    key_row = row
                    key_col = column

            elif row >= 3:
                upper_right_value = upper_right( a, row, column )
                if tmp < upper_right_value:
                    tmp = upper_right_value
                    direction = "upper_right"
                    key_row = row
                    key_col = column

        if standard_value-4 >=  row:
            under_value = under( a, row, column )
            if tmp < under_value:
                tmp = under_value
                direction = "under"
                key_row = row
                key_col = column
print( int(tmp) )
print( "column: %d / row: %d (value: %d), direction: %s" % (key_col+1, key_row+1, a[key_col][key_row], direction) )