質問をすることでしか得られない、回答やアドバイスがある。

15分調べてもわからないことは、質問しよう!

ただいまの
回答率

87.78%

Python3,pygame オブジェクト指向、自分のコードについてのアドバイス聞きたいです(詳しくは本文で)

解決済

回答 4

投稿 編集

  • 評価
  • クリップ 4
  • VIEW 3,392
python3でpygameを使っています。

質問が漠然としていて申し訳ございませんが、
君のコードは汚い、オブジェクト指向がわかってないとよく言われます。

具体的にオブジェクト指向が何なのか
そしてこの関数の少なさ、ループの多さをどう解決したらいいのでしょうか。

アドバイス頂きたいと思います。

import pygame
from pygame.locals import *
import sys
import PygameSupportLibrary
from PygameSupportLibrary import Image
import math

def color(Num):
    if Num == 1:
        return "white"
    elif Num == 2:
        return "black"

def load_image(target):
    return pygame.image.load(target).convert_alpha()

pygame.init()
pygame.display.set_caption(u"Othelo")

board = [[0 for i in range(8)] for i in range(8)]
board[3][3] = board[4][4] = 1
board[3][4] = board[4][3] = 2

turn = 1

screen_SIZE = (500,500)
screen = pygame.display.set_mode(screen_SIZE)

boardImg = load_image('pic/board.png')
whiteImg = load_image('pic/white.png')
blackImg = load_image('pic/black.png')
# 白石 = 1 , 黒石 = 2 として扱う

myfont = pygame.font.SysFont(None, 60)

def put_stone(ImgNum,PosNum1,PosNum2):
    global Img
    if ImgNum == 1:
        Img = whiteImg
    elif ImgNum == 2:
        Img = blackImg
    Pos1 = PosNum1 * 50 + 20 + (PosNum1 + 1)
    Pos2 = PosNum2 * 50 + 20 + (PosNum2 + 1)

    return screen.blit(Img,(Pos1,Pos2))

def switch_turn(p_turn):
    global turn
    if p_turn == 1:
        return 2
    elif p_turn == 2:
        return 1

def ret_stone(sta1,sta2,end1,end2,direction):
    while True:
        p_sta1 = sta1 + direction[0]
        p_sta2 = sta2 + direction[1]
        board[p_sta1][p_sta2] = turn
        if p_sta1 == end1 and p_sta2 == end2:
            print(p_sta1,p_sta2,end1,end2,direction)
            break
        return

def set_stone(x, y):
    global turn

    direction = []

# 座標からクリックされたマスを割り出す
    Rx = x - 20
    Ry = y - 20
    squ1 = math.floor(Rx / (50 + 1))
    squ2 = math.floor(Ry / (50 + 1))

    Con1 = bool     # 石があらかじめそこにないこと
    Con2 = bool     # 石に隣接すること
    Con3 = bool     # 盤面の中にあること
    Con4 = bool     # 自分の石で相手の石を挟んでいること

# すでに石が置いてないか → 正常作動
    if board[squ1][squ2] == 0:
        Con1 = True
    else:
        Con1 = False

# 石に隣接するか → listの外の数が定義されてしまう
    for i in range(-1,2):
        for j in range(-1,2):
            if board[squ1 + i][squ2 + j] == 0 or board[squ1 + i][squ2 + j] == turn:
                pass
            else:
                Con2 = True
                direction.append([i,j])

    if Con2 != True:
        Con2 = False

# ボード上にますがにますが存在するか → 正常作動
    if 0 < Rx < 409 and 0 < Ry < 409:
        Con3 = True
    else:
        Con3 = False

# 相手の石を挟んでいるか → 製作中
    for direct in direction:
        p_squ1 = squ1
        p_squ2 = squ2
        while True:
            p_squ1 = p_squ1 + direct[0]
            p_squ2 = p_squ2 + direct[1]
            if p_squ1 == 8 or p_squ1 == -1 or p_squ2 == 8 or p_squ2 == -1:
                break
            if board[p_squ1][p_squ2] == turn:
                Con4 = True
                ret_stone(squ1,squ2,p_squ1,p_squ2,direct)
                break
            else:
                pass

        if Con4 != True:
            Con4 = False

# 上記4条件を満たした時実行
    if Con1 and Con2 and Con3 and Con4 == True:
        board[squ1][squ2] = turn
        turn = switch_turn(turn)
    else:
        pass

while True:
    screen.fill((0,0,0))
    screen.blit(boardImg,(20,20))

    str_turn = myfont.render("turn=" + str(color(turn)), True, (225,225,225))
    screen.blit(str_turn, (20,434))

    for i in range(0,8):
        for j in range(0,8):
            if board[i][j] == 0:
                pass
            elif board[i][j] == 1 or 2:
                put_stone(board[i][j],i,j)

    pygame.display.update()

    for event in pygame.event.get():
        if event.type == QUIT:
            sys.exit()

        if event.type == MOUSEBUTTONDOWN and event.button == 1:
            x, y = event.pos
            set_stone(x, y)

補足(2015/8/13):オセロの盤面を作っています。

  • 気になる質問をクリップする

    クリップした質問は、後からいつでもマイページで確認できます。

    またクリップした質問に回答があった際、通知やメールを受け取ることができます。

    クリップを取り消します

  • 良い質問の評価を上げる

    以下のような質問は評価を上げましょう

    • 質問内容が明確
    • 自分も答えを知りたい
    • 質問者以外のユーザにも役立つ

    評価が高い質問は、TOPページの「注目」タブのフィードに表示されやすくなります。

    質問の評価を上げたことを取り消します

  • 評価を下げられる数の上限に達しました

    評価を下げることができません

    • 1日5回まで評価を下げられます
    • 1日に1ユーザに対して2回まで評価を下げられます

    質問の評価を下げる

    teratailでは下記のような質問を「具体的に困っていることがない質問」、「サイトポリシーに違反する質問」と定義し、推奨していません。

    • プログラミングに関係のない質問
    • やってほしいことだけを記載した丸投げの質問
    • 問題・課題が含まれていない質問
    • 意図的に内容が抹消された質問
    • 過去に投稿した質問と同じ内容の質問
    • 広告と受け取られるような投稿

    評価が下がると、TOPページの「アクティブ」「注目」タブのフィードに表示されにくくなります。

    質問の評価を下げたことを取り消します

    この機能は開放されていません

    評価を下げる条件を満たしてません

    評価を下げる理由を選択してください

    詳細な説明はこちら

    上記に当てはまらず、質問内容が明確になっていない質問には「情報の追加・修正依頼」機能からコメントをしてください。

    質問の評価を下げる機能の利用条件

    この機能を利用するためには、以下の事項を行う必要があります。

回答 4

+6

オブジェクト指向が分かるとコードがキレイになるかというと・・・そこは関係ないと思います。言った本人はコードの質がオブジェクト指向の欠如に由来している、とまでは意味して言ってないと思いますよ。
オブジェクト指向でもなんでも、汚いコードは汚いです。

仰るとおり、関数が長すぎ(=少なすぎ)です。関数ではなく、コメントで機能を分けてしまってるのではないでしょうか。例えばこれ

# ボード上にますがにますが存在するか → 正常作動
    if 0 < Rx < 409 and 0 < Ry < 409:
        Con3 = True
    else:
        Con3 = False
こーゆーのを関数にしてしまえばいいのではないでしょうか。

def valid_position(x, y):
     return 0 < x < 409 and 0 < y < 409

以下も同様です。
# 上記4条件を満たした時実行
    if Con1 and Con2 and Con3 and Con4 == True:
        board[squ1][squ2] = turn
        turn = switch_turn(turn)
    else:
        pass

def can_put(board, x, y):
    return valid_position(x, y) and is_neighbor_stone_exists(board, x, y) and ....
コメントするのではなく、名前をつけましょう。


それとグローバル変数を使いすぎです。絶対量のことではなく、この程度でグローバル変数をつかっていたら、数千行になったときに苦労しますよ。特にturnのようなゲームの進行によって値が変わるものはグローバルに置かないほうがよいです。最後まで変わらないもの、例えば画面サイズとか石の画像ならOKです。

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2015/08/13 13:52

    global変数に関してはその方にも言われてしまいました...
    やはり関数で引数(メッセージ)として送って扱うという方法がいいんですかね。
    そしてそれがオブジェクト指向というものにつながっていくんでしょうか?

    参考になりました。ありがとうございます!

    キャンセル

  • 2015/08/13 14:29

    > そしてそれがオブジェクト指向というものにつながっていくんでしょうか?

    そういう面もちょっとだけあるかもしれません。
    オブジェクト指向というより、classの利便性ですけども、
    例えば関数に小分けして全部引数にしたときに
    put_to_board(board, x, y, stone)
    get_from_board(board, x, y)
    swap_stone_on_board(board, x, y)
    みたいになったときに、すべての関数が盤面への副作用や問い合わせだと気付きます。
    主体(モデル)は駒でも座標でもなく、盤面ですよね。
    そして毎回boardを引数に渡すのが面倒です。
    classにしてしまえば
    board.put(x, y, stone)
    board.get(x, y)
    board.swap(x, y)
    みたいに引数にboardを渡さなくて済むわけです。

    最初はこんな感じでいいんじゃないでしょうか。

    キャンセル

  • 2015/08/13 21:05 編集

    なるほど!boardの部分がclass側でselfで補完されるということですね。

    使ってみます。ありがとうございました。

    キャンセル

checkベストアンサー

+5

オブジェクト指向とコードの汚さは別問題なので、
まずコードをきれいにするところから始めましょう。


    Con1 = bool     # 石があらかじめそこにないこと
    Con2 = bool     # 石に隣接すること
    Con3 = bool     # 盤面の中にあること
    Con4 = bool     # 自分の石で相手の石を挟んでいること

きれいにするには何をするかといえば、初歩的でも重要なこととして、
たとえば上の「Con1、Con2……」のような連番は止めましょう。

そうではなく、コメントで説明していることを変数名(関数名)にしましょう。
たとえば「石があらかじめそこにないこと」なら「is_empty_cell」とか。


もしかして、これへの疑問として「変数名が長くなるではないか」
「変数の命名に時間がかかるではないか」と思うかもしれません。

しかしコードが汚いと結局、デバッグにかかる時間が増える、
という形で負債を払わされるので、そこでケチらなくていいです。



# すでに石が置いてないか → 正常作動
    if board[squ1][squ2] == 0:
        Con1 = True
    else:
        Con1 = False

「正常作動」とありますが、テストの結果はコメントで済まさないで、
なるべくドキュメントやバージョン管理システムなどで管理しましょう。

テストのしやすさと関連して、コメントで分けるのではなく、
関数(やクラス)にして切り分けましょう。
関数やメソッドへの切り出しは非常に重要です。


もしかしてまた、これへの疑問として、
「短い関数をわざわざ作る必要はないのでは」
「関数を増やすとかえって回りくどいのでは」
と思うかもしれません。

しかし、関数へ切り分けることは、
たとえばif文やfor文のネストを浅くして
複雑性を減らすことにもつながります。



オブジェクト指向については、考え方だけ述べておきます。

サンプルコードにも表れていますが、
処理の流れをif文とfor文の組み合わせで表現する、
というスタイルで思考していると思います。

しかし、オブジェクト指向ではまったく考え方が異なります。

オブジェクトの組み合わせ(メッセージング)で表現します。
このオブジェクトとは対象とか目的という意味です。
処理ではなく対象や目的を中心に考えます。


それはどういうことでしょうか?
リバーシの例で考えてみましょう。

リバーシは盤上で駒をひっくり返していくゲームですよね。
そこで、「盤」「駒」「ひっくり返す」といった、
人間にとっての対象や目的をオブジェクトにモデリングします。

「盤」はさらに「マス目」に分割できますし、
「ひっくり返す」ことも「置く」「挟む」などに分解できるでしょう。

このモデリングの基本として、名前や命名(の背後の分析)が重要になってきます。
名前は単なるラベルでただの飾り(だから連番でもいい)、と思うかもしれませんが、
じつはオブジェクト指向の本質的な要素になっています。


最初に戻って、コードのきれいさとオブジェクト指向はイコールではありませんが、
たとえば命名を大事にすることは両者で共通しています。
ですからまず最初の一歩として、連番を止めるところから始めましょう。

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2015/08/13 13:33

    一つ一つ丁寧にアドバイス頂きありがとうございました

    やはり関数化やテストの結果を残すことはデバックの上でも大事そうですね...
    gitなどバージョン管理システムについても勉強を進めたいと思います。

    キャンセル

  • 2015/08/13 13:59 編集

    他の方々のご回答もとてもわかりやすかったのですが、
    LLmanさんの回答をベストアンサーに選ばせていただき、解決済みとさせていただきました

    ご回答ありがとうございました!

    キャンセル

+2

オブジェクト指向的な考え方を身に付けたいということであれば、
まずはオセロというゲームを「モデル化」することから始めてみてはいかがでしょうか?

以下、例ですが、、、

まず、オセロをやるには何が必要でしょうか?
とりあえず、盤(Board)と石(Stone)が必要ですよね?

では、Boardというクラスと、Stoneというクラスを書いてみましょう。

class Board :

class Stone :

では、オセロにおけるStoneとはなんでしょう?
仮に、
 ・黒もしくは白である
 ・ひっくり返えすことができる
としましょうか


class Stone :
  def __init__(self, initial_color):
     self.__colors = ["white","black"]
     if initial_color in self.__colors :
         self.__current_color = initial_color
     else :
         self.__current_color = self.__colors[0]
  
  def get_current_color(self):
      return self.__current_color

  def reverse(self):
      if self.__current_color == self.__colors[0] :
         self.__current_color = self.__colors[1]
      else :
         self.__current_color = self.__colors[0]

これでStoneをひっくり返すということが出来そうですね。

stone = Stone("white")

print stone.get_current_color() #->white

stone.reverse()
print stone.get_current_color() #->black

stone.reverse()
print stone.get_current_color() #->white

では、Boardとはなんでしょう?
おそらく、8かける8のマス目(Cell)を持っていて、、、
ではCellとはなんでしょう?
おそらく、石が置かれていない状態、白石が置かれた状態、黒石が置かれた状態があって、
「石を置く」という行為ができそうだから、
def set_stone(self,stone) のようなメソッドがありそうで、、、

というようにドンドンとモデル化していくのがオブジェクト指向的な考え方です。

投稿

編集

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2015/08/13 13:46

    サンプルコードなど分かりやすい例をあげていただきとても参考になりました!

    モデル化という概念は自分の中になく新しい考え方が理解できました。

    ご回答ありがとうございました。

    キャンセル

+2

LLmanさんがサラッと書かれてる「メッセージ」って概念は、意外とポイントになる気がしています…。

メッセージ脳の作り方
メッセージ指向のイロハ

オブジェクト指向に興味があったら責務やメッセージの概念はしっかり押さえていないと、変な方向に考えが向いてしまうかもしれません。
  • オブジェクトを使うには、メッセージを送る。
  • オブジェクトには、担うべき責務がある。
当たり前だ。と思われそうですが、その意味を忘れずにいればオブジェクト指向って…?とか惑わずに済みます。

そのあとは…
機能の切り分け(責務の分担)の問題について、大いに悩みましょうw

投稿

  • 回答の評価を上げる

    以下のような回答は評価を上げましょう

    • 正しい回答
    • わかりやすい回答
    • ためになる回答

    評価が高い回答ほどページの上位に表示されます。

  • 回答の評価を下げる

    下記のような回答は推奨されていません。

    • 間違っている回答
    • 質問の回答になっていない投稿
    • スパムや攻撃的な表現を用いた投稿

    評価を下げる際はその理由を明確に伝え、適切な回答に修正してもらいましょう。

  • 2015/08/13 13:28

    >責務やメッセージの概念
    オブジェクト指向に関する理解が変わり、深まりました。
    おかげで一段階ステップアップできたと思います。

    回答ありがとうございました。

    キャンセル

15分調べてもわからないことは、teratailで質問しよう!

  • ただいまの回答率 87.78%
  • 質問をまとめることで、思考を整理して素早く解決
  • テンプレート機能で、簡単に質問をまとめられる

関連した質問

同じタグがついた質問を見る