関数のコメントが細かく多いクラスや関数 1つ 1 つにコメントを細かく多く入れる事を正しいと勘違いしている人は多くいますまた学校によってはその様な指導をする所もありますしかしプログラムが分かり易ければコメントを書く必要性は低く多いと逆に読みにくくなります特に関数の説明をするコメントの

Size: px

Start display at page:

Download "関数のコメントが細かく多いクラスや関数 1つ 1 つにコメントを細かく多く入れる事を正しいと勘違いしている人は多くいますまた学校によってはその様な指導をする所もありますしかしプログラムが分かり易ければコメントを書く必要性は低く多いと逆に読みにくくなります特に関数の説明をするコメントの"

りさこおとべ
7 years ago
Views:

1 悪いプログラムの書き方今回は応募作品中に見られる初心者にありがちな悪いプログラムの書き方について例をあげて説明したいと思います悪い書き方これだけではなくまだ沢山ありますので更に本を読んだりして勉強して下さいまた書き方について更に学びたい場合はコーディングルールをみてみると良いでしょう会社やチームごとに細かな違いがありますがコーディングルールはどの様にすれば間違いが少なく無駄が少なく統一的な書き方が出来るかを考えて作られた規約ですネット上では様々なコーディングルールが公開されています Google で "C++ コーディングルール " を検索してみて下さいローマ字表記を使用している少数ですがプログラム中にローマ字表記を使う人がいますその人たちは例外なくプログラムの書き方が良くありませんので評価する側はローマ字表記を見た時点で初心者と判断しますプログラムでは英語表記が基本です英単語は辞書ですぐ調べられるので必ず英語を使って下さい int teki_suu; int zikan; Foo* Kensaku( const char* namae ); int enemy_num; int time; Foo* Find( const char* name ); 表記が統一されていない会社説明会のスライドでも説明しましたが書き方が統一されていないプログラムは読みにくくチーム制作ではバグの原因になります大文字小文字の混在複数単語の表記順番が違うといった間違いが多いです以下の例ではスピードを表す単語が Spd, spd,speed の 3 つがあり前に動詞が付いたり付かなかったりしていますまた変数名の開始が大文字だったり小文字だったりしています class Player Vector3 pos; float Rot; float jumpspd; float speedx; float speedy; float enemy_spd; int Action; int ActCount; int ActCntTemp; ; class Player Vector3 pos; float rot; float jumpspd; float movexspd; float moveyspd; float enemyspd; int action; int actcnt0; int actcnt1; ;

2 関数のコメントが細かく多いクラスや関数 1つ 1 つにコメントを細かく多く入れる事を正しいと勘違いしている人は多くいますまた学校によってはその様な指導をする所もありますしかしプログラムが分かり易ければコメントを書く必要性は低く多いと逆に読みにくくなります特に関数の説明をするコメントの内容の多くは関数自体を見れば分かるので書く必要がありません例えば Draw() 関数の説明に描画すると書いてあっても無意味ですシンプルなコメントを心がけましょうそして分かりにくい動作をする場合や関数の呼び出しに注意するべき事項がある場合にコメントを入れましょう例えば以下の様な動作が単純な関数では書かなくても意味が理解できますまた引数も分かりやすい名前を付ける事で分かりやすく出来ます /******************************************************************************/ /* 関数名 : Stage::Clear() 書いてあるのでいらない説明 : ステージの初期化用関数関数名から意味がわかるので必要ない引数 1 : ステージ番号なるべく引数の変数名で意味がわかる様にしておく戻り値 : なし担当 : 宇佐見バージョン管理ツールをつかえば誰が何を作成変更したかわかる作成日 : 2014/06/06 バージョン管理ツールをつかえば誰が何を作成変更したかわかる */ /******************************************************************************/ void Stage::Initialize( int n ) m_ stageid = n; /******************************************************************************/ /* 関数名 : Stage::Draw() 説明 : ステージを描画する引数 1 : なし戻り値 : なし担当 : 宇佐見作成日 : 2014/06/06 */ /******************************************************************************/ void Stage::Draw() m_stageobject.draw(); void Stage::Initialize( int stageid ) m_stageid = stageid; void Stage::Draw() m_stageobject.draw(); 即値を多用する会社説明会のスライドでも説明しましたが調整や変更される可能性が高い箇所に直に数値を指定してはいけません以下の例では三次元のマップデータを生成していますしかしこれでは固定のサイズのマップデータしか生成できません class MapBlock int m_blocktbl[10][30][30]; public: void Initialize(); ; void MapBlock::Initialize() for( int y = 0; y < 10; ++y ) for( int z = 0; z < 30; ++z ) for( int x = 0; x < 30; ++x ) m_blocktbl[y][z][x] = BLOCK_SPACE; m_blocktbl[1][ 2][ 2] = BLOCK_DOOR; m_blocktbl[3][15][ 4] = BLOCK_START; m_blocktbl[7][ 4][28] = BLOCK_GOAL;

3 m_blocktbl[6][14][18] = BLOCK_SW1; m_blocktbl[0][22][ 6] = BLOCK_SW2; プログラムが長くなりますが以下の様に大きさが変わっても問題無く動くようにするべきですそして生成する際の大きさやアイテムの配置はデータ化していますこの様に設定部分をデータ化する事で後で容易に変更できるようになりますデータ部分以外に 0 以外の即値がない点が重要です struct MapItem int m_x; int m_y; int m_z; int m_type; ; struct MapData int m_xsize; int m_zsize; int m_ysize; const MapItem* m_itemtbl; ; class MapBlock int* m_blocktbl; int m_xsize; int m_zsize; int m_ysize; int m_xzsize; int m_xzysize; public: void Initialize( const MapData& data ); void SetItem( const MapItem& item ); ; void MapBlock::Initialize( const MapData& data ) m_xsize = data.m_xsize; m_zsize = data.m_zsize; m_ysize = data.m_ysize; m_xzsize = m_xsize * m_zsize; m_xzysize = m_xzsize * m_ysize; m_blocktbl = new int[m_xzysize]; for( int y = 0; y < m_ysize; ++y ) for( int z = 0; z < m_zsize; ++z ) for( int x = 0; x < m_xsize; ++x ) int n = ( y * m_xzsize ) + ( z * m_xsize ) + x; m_blocktbl[n] = BLOCK_SPACE; for( int i = 0; i < data.m_itemtbl[i].m_type!= BLOCK_SPACE; ++i ) SetItem( data.m_itemtbl[i] ); void MapBlock::SetItem( const MapItem& item ) if( item.m_x < 0 item.m_x >= m_xsize ) return; if( item.m_z < 0 item.m_z >= m_zsize ) return; if( item.m_y < 0 item.m_y >= m_ysize ) return; int n = ( item.m_y * m_xzsize ) + ( item.m_z * m_xsize )+ item.m_x; m_blocktbl[n] = item.m_type; void CreateMap00() const MapItem mapitem00[] = 2, 2, 1, BLOCK_DOOR, 15, 4, 3, BLOCK_START, 4, 28, 7, BLOCK_GOAL, 14, 18, 6, BLOCK_SW1, 22, 6, 0, BLOCK_SW2, 0, 0, 0, BLOCK_SPACE, ;

4 const MapData mapdata00 = 30, 30, 10, mapitem00 ; MapBlock mapblock; mapblock.initialize( mapdata00 ); 1 つの式に [] 演算子 -> 演算子. 演算子が何個も使われているオブジェクトを参照する為に使われる [] 演算子 -> 演算子. 演算子が連なって使われる場合はプログラムの構造化が出来ていないか構造体やクラスの設計に問題がある場合が殆どです処理ごとに関数を分けて下さいそうするとこれらの演算子を多用する事は無くなります以下の例では Game クラスの中に Player クラスの処理が入っていますその為 Player クラスやその先のオブジェクトを参照する為の演算子を多用します void Game::UpdatePlayer() for( int i = 0; i < PLAYER_NUM; ++i ) if( g_playertbl[i].getactionflag() == ACT_ATTACK ) Vector3 dirc = g_playertbl[i].getweapon()->getdirction(); Vector3 pos = g_playertbl[i].getposition(pl_arm_r); g_playertbl[i].getweapon()->createbullet( pos, dirc ); if( g_playertbl[i].getactionflag() == ACT_GUARD ) // ガードの処理 Player クラスに対する処理は Player クラスの関数で処理を行う様にしますまたプレイヤーの条件による処理の分岐とプレイヤー起こす行動で動作の意味が違いますのでこれも関数を分けます void Game::UpdatePlayer() for( int i = 0; i < PLAYER_NUM; ++i ) g_playertbl[i].update(); void Player::Update() if( GetActionFlag() == ACT_ATTACK ) Attack(); if( GetActionFlag() == ACT_GUARD ) Guard(); void Player::Attack() Weapon* weapon = GetWeapon(); Vector3 dirc = weapon->getdirction(); Vector3 pos = GetPosition(PL_ARM_R); weapon->createbullet( pos, dirc ); void Player::Guard() // ガードの処理関数 1 つの行数が長い関数の行数が多い場合 1 つの関数に処理を詰め込みすぎている場合が殆どです

5 長くなる場合は機能ごとに関数を分けて下さいそうするとこれらの長い関数になる事は無くなります基本 100 行以内に収めましょう以下の例ではプレイヤーの移動処理全てを 1 つの関数にまとめて書いてしまっていますこの様なパターンはよく見かけますプレイヤーの状態判定と行動処理ステージとの衝突判定が全て入っています void Game::UpdatePlayer() const float limit = 0.0f; D3DXVECTOR3 vec; if( player.getflyflag() == false && player.getactiveflag() == false ) D3DXVECTOR3 movepos; if( IsInputKey(KEY_UP) ) bool ishit = false; D3DXVec3TransformCoord(&vec,&D3DXVECTOR3( 0, 0, 1 ), &player.getrot() ); D3DXVec3Normalize( &vec, &vec ); for(int i = 0; i < stage.getobjmax(); i++ ) D3DXVECTOR3 dist = player.getpos() - stage.getobj(i)->getpos(); if( D3DXVec3Length( &dist ) <= 12 ) short collisionid = stage.getobj(i)->getcollisionid(); if(collisionid!=-1) if( player.frontray( xobj.getobj( collisionid )->GetMesh(), &stage.getobj(i)->getmatrix(), limit, &movepos ) == true ) ishit=true; else if( player.frontray( xobj.getobj( stage.getobj(i)->getobjnum() )->GetMesh(), &stage.getobj(i)->getmatrix(), limit, &movepos ) == true) ishit=true; if( ishit == true ) bool bflg = false; D3DXVec3TransformCoord( &vec, &D3DXVECTOR3( 0, 0, -1 ), &player.getrot() ); D3DXVec3Normalize( &vec, &vec ); for( int i = 0; i < stage.getobjmax(); i++ ) D3DXVECTOR3 dist = player.getpos() - stage.getobj(i)->getpos(); if( D3DXVec3Length( &dist ) <= 12 ) if( bflg == false ) D3DXVECTOR3 plpos = player.getpos() + movepos; player.initpos( plpos.x, player.gety(), plpos.z ); else if(player.getjumpflg()==false) player.setpos( 0, 0, 0.5f ); else player.setpos( 0, 0, 0.25f ); if( IsInputKey( KEY_DOWN ) ) if( IsInputKey( KEY_LEFT ) )

6 if( IsInputKey( KEY_RIGHT ) ) if( IsInputKey( KEY_SPACE ) ) この様な場合衝突判定は Stage クラスの関数にプレイヤーの処理は Player クラスの関数に分け更に Player クラスの関数は条件判定処理と行動処理で更に関数に分けますこの様に関数 1 つ 1 つが何の処理を担当しているか明確に区分する事が重要です const float limit = 0.0f; bool Stage::CollisionPlayer( Player& player, D3DXVECTOR3* movepos ) D3DXVECTOR3 vec; D3DXVec3TransformCoord(&vec,&D3DXVECTOR3( 0, 0, 1 ), &player.getrot() ); D3DXVec3Normalize( &vec, &vec ); for( int i = 0; i < GetObjMax(); i++ ) SMesh* meshobj = stage.getobj(i); D3DXVECTOR3 dist = player.getpos() - meshobj->getpos(); if( D3DXVec3Length( &dist ) <= 12 ) short collisionid = GetObj(i)->GetCollisionId(); LPD3DXMESH d3dmesh = collisionid!= -1? xobj.getobj( collisionid )->GetMesh() : xobj.getobj( meshobj->getobjnum() )->GetMesh() ; if( player.frontray( d3dmesh, &meshobj->getmatrix(), limit, movepos ) ) return true; return false; bool Stage::CollisionPlayer2nd( Player& player, D3DXVECTOR3* movepos ) D3DXVECTOR3 vec; D3DXVec3TransformCoord( &vec, &D3DXVECTOR3( 0, 0, -1 ), &player.getrot() ); D3DXVec3Normalize( &vec, &vec ); for( int i = 0; i < GetObjMax(); i++ ) SMesh* meshobj = stage.getobj(i); D3DXVECTOR3 dist = player.getpos() - meshobj->getpos(); if( D3DXVec3Length( &dist ) <= 12 ) return false; void Player::MoveForward() D3DXVECTOR3 movepos; bool ishit = stage.collisionplayer( *this, &movepos ); if( ishit == true ) bool ishit2 = stage.collisionplayer2nd( *this, &movepos ); if( ishit2 == false ) D3DXVECTOR3 plpos = GetPos() + movepos; InitPos( plpos.x, GetY(), plpos.z );

7 else if( GetJumpFlg() == false ) SetPos( 0, 0, 0.5f ); else SetPos( 0, 0, 0.25f ); void Player::Update() if( GetFlyFlag() == false && GetActiveFlag() == false ) if( IsInputKey( KEY_UP ) ) MoveForward(); if( IsInputKey( KEY_DOWN ) ) MoveBack(); if( IsInputKey( KEY_LEFT ) ) TurnLeft(); if( IsInputKey( KEY_RIGHT ) ) TurnRight(); if( IsInputKey( KEY_SPACE ) ) Jump(); // File game.h void Game::UpdatePlayer() player.update(); グローバル変数スタティック変数を多用するゲーム全体で使用する変数を利用するときグローバル変数スタティック変数を使わないようにしましょう初心者はこのような変数を 1 つのファイルに設定値をまとめてそれを参照するための global.h を定義するといった傾向があります以下の様な例ではゲームでの設定値は Game クラスのオブジェクトにグラフィックデバイス関連の値は GraphicDevice クラスにと機能ごとにクラスを作りそのメンバとして定義する様にして下さい // File global.h extern LPDIRECT3DDEVICE g_d3ddevice; extern int g_nowstageid; extern int g_player1id; extern int g_player2id; extern int g_playtime; extern bool g_debugflag; // File main.cpp LPDIRECT3DDEVICE g_d3ddevice; // グラフィックデバイスへのアクセス int g_nowstageid; // 現在のステージ番号 int g_player1id; // 1Pのプレイヤー番号 int g_player2id; // 2Pのプレイヤー番号 int g_playtime; // プレイ残り時間 bool g_debugflag; // デバッグ機能ありorなし // File stage.h class Stage int m_id; ; // File graphicdevice.h class GraphicDevice LPDIRECT3DDEVICE m_device; ; // File player.h class Player int m_id; ; // File game.h

8 class Game int m_debugflag; int m_playtime; ; // それぞれのクラスは必要な段階で new をして確保する同じ処理を何度も呼び出す以下の例では関数は整頓されているように見えますが条件判断文の条件と関数の引数が違うだけで同じ関数呼び出しが並んでいますこのようなプログラムはデータと処理に分けることが出来ます void Enemy::DropItem() int temp = GetRand(19); switch( kind ) case MANTIS: if( temp < 10 ) MakeItem( Item::POINT_LOW,position ); else if( temp == 10 ) MakeItem( Item::HP_MINI,position ); else if( temp < 14 ) MakeItem( Item::BOM_MINI,position ); case APPLE: if( temp < 8 ) MakeItem( Item::POINT_HIGH,position ); else if( temp < 12 ) MakeItem( Item::SCORE,position ); else if( temp < 16 ) MakeItem( Item::RAPID,position ); else MakeItem( Item::HP_BIG,position ); case MISSILETANK: if( temp < 3 ) MakeItem( Item::HP_MINI,position ); else if( temp < 4 ) MakeItem( Item::HP_BIG,position ); else if( temp < 9 ) MakeItem( Item::BOM_MINI,position ); else if( temp < 14 ) MakeItem( Item::BOM_BIG,position ); 違う部分 ( 条件判断文の条件と関数の引数 ) をデータとして分けますこの部分は将来ファイルから読み込ませたりエディタで値を変更させられるようにしたりといった変更を簡単に出来るようになりますまたデータ部分が増えてもプログラムは増えることはありません static Item::Type GetDropItem( const DropItemTbl* dropitemptr ) int rate = GetRand(19); for( ; dropitemptr->rate!= 0; ++dropitemptr ) if( rate < dropitemptr->rate ) return dropitemptr->itemtype; void Enemy::DropItem() const DropItemTbl dropitemtbl_mantis[] = 10, Item::POINT_LOW, 11, Item::HP_MINI, 14, Item::BOM_MINI, 0, Item::NONE, ; const DropItemTbl dropitemtbl_apple[] = 8, Item::POINT_HIGH, 12, Item::SCORE, 16, Item::RAPID, 0, Item::HP_BIG, ; const DropItemTbl dropitemtbl_missiletank[] = 10, Item::POINT_LOW, 11, Item::HP_MINI, 14, Item::BOM_MINI, 0, Item::NONE, ; const DropItemTbl* dropitemtbl[] = dropitemtbl_mantis, dropitemtbl_apple, dropitemtbl_missiletank, ; // テーブル数はテーブルから求める const int tblsize = sizeof(dropitemtbl)/sizeof(dropitemtbl[0]);

9 if( kind < 0 kind >= tblsize ) return; Item::Type itemtype = GetDropItem( dropitemtbl[kind] ); if( itemtype!= Item::NONE ) MakeItem(itemType,position); 変数の確保定数に const 指定を行わない定数には必ず const を付けましょう以下の例では関数内にデータテーブルを持っていてそれが書きかえられないにも関わらず const を付けていませんこの様な箇所が多いと変数や定数の区別が付いていない初心者と判断されてしまいます void StageBlock::CheckCorner( int xpos, int ypos ) int checktbl[][2] = 1,1, 1,-1, -1,-1, -1,1 ; unsigned int result = 0; for( int i = 0; i < 4; ++i ) result = CheckPos( xpos + checktbl[i][0], ypos + checktbl[i][1] ); void StageBlock::CheckCorner( int xpos, int ypos ) const int checktbl[][2] = 1,1, 1,-1, -1,-1, -1,1 ; unsigned int result = 0; for( int i = 0; i < 4; ++i ) result = CheckPos( xpos + checktbl[i][0], ypos + checktbl[i][1] );

Microsoft PowerPoint - CproNt02.ppt [互換モード]

Microsoft PowerPoint - CproNt02.ppt [互換モード] 第 2 章 C プログラムの書き方 CPro:02-01 概要 C プログラムの構成要素は関数 ( プログラム = 関数の集まり ) 関数は, ヘッダと本体からなる使用する関数は, プログラムの先頭 ( 厳密には, 使用場所より前 ) で型宣言 ( プロトタイプ宣言 ) する関数は仮引数を用いることができる ( なくてもよい ) 関数には戻り値がある ( なくてもよい void 型 ) コメント