ページ 1 / 1
綺麗なソースの書き方
Posted: 2012年1月13日(金) 18:46
by Cr
最近、少しずつ長いプログラムを書くようになってきて、自分で書いたのを後で見て何やってるか分かんないことがしばしばあります
やっぱり、読んでて汚くて…
そこで、みなさんがソースを書くとき綺麗にするために気をつけてることがあれば教えてください!
それと、下にこの間作ったプログラムを載せるので、ダメだし等お願いします。
動きとしては、漢文に返り点をつけるプログラムで
漢文の文字数入力→漢文の読む順番を半角で区切って入力→上から順番につける返り点を表示
というものです。
今回は完成してから変数に意味を持たせるなど、気をつけて推敲してみました。(たぶんまだ駄目な気がしますが…)
コード:
#include <iostream>
#include <iomanip>
#include <vector>
#include <string>
using namespace std;
string moji[8][10]={{"上","中","下"},
{"一","二","三","四","五","六","七"},
{"上","下"},
{"甲","乙","丙","丁","戊","己","庚","辛","任","癸"},
{"天","地","人"},
{"元","亨","利","貞"},
{"春","夏","秋","冬"},
{"木","火","土","金","水"}
};
struct onetwo{
int revel;
int nest;
};
typedef vector<int> vec_i;
typedef vector<onetwo> vec_onetwo;
void solve(int n,vec_i read_order,vec_i num1,vec_i num2,vec_onetwo *edge);
void print_vector(vec_i vec);
void set_num1(vec_i *num1,vec_i num2,int n);
int main(){
vector<int> read_order;
vector<int> num1,num2; //num1は相対的な距離 num2はx回目に読む文字がy番目にあるのをnum2[x]=y
cout <<"漢文の文字数を入力"<<endl;
int n;
cin >>n;
num1.resize(n);
num2.resize(n);
onetwo start;
start.revel=-1;
start.nest=-1;
vector<onetwo> edge(n,start);
cout << "漢文の読む順番を半角スペースで区切って入力"<<endl;
for(int i=0;i<n;i++){
int a;
cin >> a;
read_order.push_back(a-1);
num2[a-1]=i;
}
set_num1(&num1,num2,n);
solve(n,read_order,num1,num2,&edge);
for(int i=0;i<n;i++){
if(i!=n&&num1[i+1]==-1) //次に進む場所が一つ前なら
cout <<"レ"; //レ点
else
cout <<" ";
if(edge[i].nest!=-1){ //nestが初期値と違ったら
cout<<moji[edge[i].nest][edge[i].revel]; //一二点系返り点表示
if(num1[i]==1) //一二点の後、行き先が一つ下なら
cout <<"|"; //ハイフン表示
else
cout <<" ";
}else{
cout <<" ";
}
cout<<":"<<endl;
}
return 0;
}
void solve(int n,vec_i read_order,vec_i num1,vec_i num2,vec_onetwo *edge){
int revel=0;
int nest=1;
vector<int>check_same_nest;
for(int i=0;i<n;i++){
if(num1[ num2[i] ]<-1){ //2個以上もどるとき ex: 一,上
onetwo now;
now.revel=revel; //revel引き継ぎ
for(int now=num2[i]-1 ; now>num2[i+1] ; now--){ //返り点の間に別の返り点があるかチェック
if((*edge)[now].nest>=nest){ //自分以上にnestの高いのがいれば
nest = (*edge)[now].nest + 1; //それ+1を代入
}
}
check_same_nest.push_back(num2[i]); //通った点を記録
if((*edge)[ check_same_nest[0] ].nest<nest){ //昔のネストが今より低かった時
for(int i=0 ; i<check_same_nest.size() ; i++){
(*edge)[ check_same_nest[i] ].nest=nest; //過去の地点に今のネストを代入
}
}
if(nest==2){ //上下点の処理
switch(revel){
case 1: //上中下点へ
nest=0;
break;
case 2: //甲乙点へ
nest=3;
for(int i=0;i<2;i++){
(*edge)[ check_same_nest[i] ].nest=3;
}
break;
}
}
now.nest=nest; //ネストを代入
(*edge)[ num2[i] ]=now; //答えに代入
revel++;
now.revel=revel; //返る場所にも返り点をつける ex:二,下
(*edge)[ num2[i+1] ]=now;
}else{ //返り点が連続しなかったとき
check_same_nest.clear(); //データをそれぞれ初期化
revel=0;
nest=1;
}
}
}
void set_num1(vec_i *num1,vec_i num2,int n){
for(int i=0;i<n-1;i++){
(*num1)[ num2[i] ]=num2[i+1]-num2[i];//相対距離を代入
}
(*num1)[ num2[n-1] ]=0;//終点には0を代入
}
void print_vector(vec_i vec){//与えられた配列の全要素を表示
for(int i=0;i<vec.size();i++)cout <<setw(2)<<internal<< vec[i]<<endl;
cout <<endl;
}
}
実行結果
コード:
漢文の文字数を入力
12
漢文の読む順番を半角スペースで区切って入力
1 2 12 9 3 6 7 5 4 8 11 10
:
:
乙 :
下 :
:
二|:
:
レ一 :
:
上 :
レ甲 :
:
Re: 綺麗なソースの書き方
Posted: 2012年1月13日(金) 20:15
by 史上最悪のデスペナ
Cr さんが書きました:みなさんがソースを書くとき綺麗にするために気をつけてることがあれば教えてください!
これに関していえば、Crさんみたいなインデントの仕方に+して
forとかstructとかみたいに{}で括るもの(enumを除く)は
と、書きますね。
それとか、
のように、変数を宣言した後の初期化はインデント入れたり、「+」、「ー」、「=」などの演算子の前後に空白を入れたり、
関数の引数は
func( a, b, c, d )
と空白を入れたりします。( func1( func2(a,b), b, c, d ) のように引数の中に関数が入る場合は引数に使う関数は空白入れませんが。 )
あとは、可能な限り処理をまとめて{}で括ります。
例えば、こんな感じです。
コード:
//自機
{
Player.Move();
Player.Motion();
}
//MOB
{
MOB.Move();
MOB.Motion();
}
//戦闘
{
Player.Attack();
MOB.Attack();
Player.Damage();
MOB.Damage();
}
勿論、{}の中でもさらに似た関数ごとにまとめて空行で区切ります。
あとは、
コード:
#includeなど
////////////////////////////
スタート画面
/////////////////////////////
ロード画面
/////////////////////////////
メインループ
////////////////////////////
みたいなこともします。
あとは、変数の名前ですね。
分かりやすいように、
自キャラのダメージならMyDamageみたいに、頭文字を大文字にして英語化したり、
自機に関するクラスならC_PlayerのようにC_(構造体ならS_)をつけたりしています。
これぐらいですかね?
Re: 綺麗なソースの書き方
Posted: 2012年1月13日(金) 21:54
by Cr
回答ありがとうございます
確かにこの方が空白が目立って一個一個分かりやすいですね
初期化でインデントするってはじめてみました
空白はプログラム上では意味を持たないって分かってるくせにforとか以外でインデントをする発想が無かった…
ちょうど
コード:
onetwo start;
start.revel=-1;
start.nest=-1;
vector<onetwo> edge(n,start);
のところで困ってたんです
使わせていただきますね
史上最悪のデスペナ さんが書きました:
「+」、「ー」、「=」などの演算子の前後に空白を入れたり、
演算子の空白、忘れがちなんですよね…
注意して書いたさっきのですら忘れてるし……
史上最悪のデスペナ さんが書きました:コード:
//自機
{
Player.Move();
Player.Motion();
}
//MOB
{
MOB.Move();
MOB.Motion();
}
//戦闘
{
Player.Attack();
MOB.Attack();
Player.Damage();
MOB.Damage();
}
これいいですね!
処理の説明に関数を用いようかと考えていたのですが、こういう方法もあるわけですね
史上最悪のデスペナ さんが書きました:コード:
#includeなど
////////////////////////////
スタート画面
/////////////////////////////
ロード画面
/////////////////////////////
メインループ
////////////////////////////
分割されてるとこが分かりやすいですね
僕がいつもやってるコンテストとか用のプログラムだと…入力 計算 出力 で分けるのかな?
あ、でも出力なんかそんなに無いから 入力 計算1 計算2? その辺はものによるか……
史上最悪のデスペナ さんが書きました:
自キャラのダメージならMyDamageみたいに、頭文字を大文字にして英語化したり、
頭文字大文字で英語化ですか
自分は_でつないでました
かわりに構造体やクラスを表す時に使うんですね
勉強になります
どうもありがとうございました
Re: 綺麗なソースの書き方
Posted: 2012年1月13日(金) 22:19
by beatle
変数宣言の後にインデントですか?今まで見たことがありませんでした.もしそんなソースみたら,今までの僕なら「インデントさえ知らないやつか」という目で見ていたと思います.
ブロックを構成する{}ですが,{の前に改行を入れるか入れないか,それはどちらも流派としてあるので好きな方でいいかと.一応,昔はディスプレイの表示行数が少なかったため,forと同じ行に{を書く方が流行っていたとか.
変数名の頭文字を大文字にするのは,この掲示板以外ではあまり見たことがありません.DXライブラリ周辺の人たちが好んで使っているような印象を持っています.
Java界隈ではcamelCase,C/C++界隈ではunder_scoreが多いと思います.
ああ,関数名は先頭大文字でCamelCaseの場合がありますね.
Re: 綺麗なソースの書き方
Posted: 2012年1月13日(金) 22:21
by beatle
Re: 綺麗なソースの書き方
Posted: 2012年1月13日(金) 23:16
by softya(ソフト屋)
書く人によって様々なスタイルがありますが私も変数宣言後にインデントするのは避けたいですね。
でコードスタイルの話になっていますが変数名も大事です。
vec_onetwo;とかonetwoが何を表しているのかさっぱりわかりません。あとrevelはレベル=levelのことだと思いますが自信がない英単語は出来るだけ辞書で調べましょう。覚え間違いしていることも良くあります。num1,num2と言う様な意味を持たない変数名も厳禁です。ほかはvecを付けているのにつけていない統一性のなさも問題ですね。
Re: 綺麗なソースの書き方
Posted: 2012年1月14日(土) 00:04
by Fimbul
例外もありますが、私は変数名を 型 + 表す物の修飾語 + 表す物 という形にしています。
例えばint型でザコ敵のHPを表すなら nEnmHP、float型で中ボスのスピードを表すなら fMidBossSpd と言った具合です。
上の二つを nHP、fSpd とすると、何のHP、Spdなのか分からなくなってしまうので修飾語をつける様にしています。
便乗ですが、変数の頭を型にする規則は賛否両論あるみたいで、皆さんはどの様にしているのでしょうか。
Re: 綺麗なソースの書き方
Posted: 2012年1月14日(土) 00:06
by Cr
>beatleさん
変数宣言の後にインデントですか?今まで見たことがありませんでした.もしそんなソースみたら,今までの僕なら「インデントさえ知らないやつか」という目で見ていたと思います.
そうですか… うーむ結構いいアイデアかと思ったのですか
初期化の部分のみにインデントってことですよね
少なくとも人が見るようなコードで書くのはやめにしようかな…
ブロックを構成する{}ですが,{の前に改行を入れるか入れないか,それはどちらも流派としてあるので好きな方でいいかと.
どちらでもいいんですね
僕は良く「空白行が無さ過ぎて読みづらい」と言われるので、スペースを空けるためにも史上最悪のデスペナさんが紹介してくれた方で今後書いてみようと思います
Java界隈ではcamelCase,C/C++界隈ではunder_scoreが多いと思います.
ああ,関数名は先頭大文字でCamelCaseの場合がありますね.
言語によっても違うのですか…
とりあえず、
変数名 under_score
関数名 CamelCase
で統一してみます
おぉ!細かい!!
明日から早速読んでみます
どうもありがとうございました
>softya(ソフト屋)さん
書く人によって様々なスタイルがありますが私も変数宣言後にインデントするのは避けたいですね。
やはりそうですか…
vec_onetwo;とかonetwoが何を表しているのかさっぱりわかりません。
一二点の一二なのですが…
itiniとしても何やら読みづらいし、かといって他の読み方知らないし……
英訳出来ないときはどうするんでしょう?
itini_tenとか?
これならまだ分かりますかね?
あとrevelはレベル=levelのことだと思いますが自信がない英単語は出来るだけ辞書で調べましょう。覚え間違いしていることも良くあります。
lでしたか…
確かに書いてる途中にrevelとlevelが混じってコンパイラーに怒られました
levelが正しかったのですね… 辞書ひくようにします
num1,num2と言う様な意味を持たない変数名も厳禁です。
うぅん…
これも考えては見たのですが思い浮かばずに…
添え字:場所 値:次に飛ぶ場所-今の場所 がnum2で、
添え字:場所 値:何回目に読むか がread_orderなのですが
添え字:何回目に読むか 値:場所 とした配列がnum1です。
みなさんならなんて名付けますか?
それと、なかなか良い変数の名前が浮かばないとき、どうしてますか?
ほかはvecを付けているのにつけていない統一性のなさも問題ですね。
そんな綺麗にまとめるようなことしたかなぁ…と思ったら
コード:
typedef vector<int> vec_i;
typedef vector<onetwo> vec_onetwo;
これのことをおっしゃってるのかな?
関数を定義するときにvector<int>をいちいち打ってるのがめんどくさくなったので省略化しました。
構造体にS_○○クラスにC_○○という名前をつけるように、vectorにもv_○○とつけるべきですか?
それと、もう2つ質問です
今回のプログラムでは動きができてから、綺麗に直そうと思って綺麗に直していきました。
ですが、最初からこの状態のプログラムを書けと言われたらできる気がしません。
変数名等はともかく、どこを関数化するかや、同じ動作でもどう書くのが一番読みやすいか等など…
皆さんはどうやっていますか?
コメントはどれぐらい書きますか?
以前は全く書かなかったのですが、最近気をつけてちょくちょく書くようにしています。
「ソースコードを見て分かるものは書かなくてもいい」ということも聞いたことがありますが、そもそもどのレベルまでが「ソースコードを見て分かるもの」なのか分かりません。
Re: 綺麗なソースの書き方
Posted: 2012年1月14日(土) 00:11
by Cr
>Fimbul さん
すみません、返信書いてる間に書いてくださったようで、見過ごしていました
例外もありますが、私は変数名を 型 + 表す物の修飾語 + 表す物 という形にしています。
なるほど…似たようなものが多くなってくると確かにこうして行かないと処理できませんね
便乗ですが、変数の頭を型にする規則は賛否両論あるみたいで、皆さんはどの様にしているのでしょうか。
僕も気になります。
個人的にはvectorなど特徴的なものでなければ型は無くてもいいのではないかという気もしますが、どうなのでしょう?
Re: 綺麗なソースの書き方
Posted: 2012年1月14日(土) 00:52
by softya(ソフト屋)
プログラムのアルゴリズム自体を理解していないのですが、
Cr さんが書きました:一二点の一二なのですが…
itiniとしても何やら読みづらいし、かといって他の読み方知らないし……
英訳出来ないときはどうするんでしょう?
itini_tenとか?
これならまだ分かりますかね?
何の12点なのかという視点が抜けています。
point12でもTwelvePointでも良いのですが更に何のと言う意味が欲しいです。
Cr さんが書きました:うぅん…
これも考えては見たのですが思い浮かばずに…
添え字:場所 値:次に飛ぶ場所-今の場所 がnum2で、
添え字:場所 値:何回目に読むか がread_orderなのですが
添え字:何回目に読むか 値:場所 とした配列がnum1です。
みなさんならなんて名付けますか?
それと、なかなか良い変数の名前が浮かばないとき、どうしてますか?
添え字:場所 値:次に飛ぶ場所-今の場所 PointToNextPoint
添え字:場所 値:何回目に読むか PointToReadOrder
添え字:何回目に読むか 値:場所 ReadOrderToPoint
でしょうか。
Cr さんが書きました:
vectorにもv_○○とつけるべきですか?
蓄える物次第ですが無いより有ったほうが分かりやすいなら付けるべきです。
PointToNextPoint、 PointToReadOrder、ReadOrderToPointと付けるなら不要だと思います。
ただのnum1では直感的にintだとは思うでしょうが配列だとは思いません。
Re: 綺麗なソースの書き方
Posted: 2012年1月14日(土) 01:07
by softya(ソフト屋)
Cr さんが書きました:それと、もう2つ質問です
今回のプログラムでは動きができてから、綺麗に直そうと思って綺麗に直していきました。
ですが、最初からこの状態のプログラムを書けと言われたらできる気がしません。
変数名等はともかく、どこを関数化するかや、同じ動作でもどう書くのが一番読みやすいか等など…
皆さんはどうやっていますか?
作っている最中に関数化や読みやすくするにはどうするかを常に考えています。
それでもやはり読みづらい物が出来る場合もあるので、それは後で見なおしてリファクタリングします。
とりあえず関数が30~40行を超えそうならヤバイかなぁとか、同じような処理が出てきたら関数化しようとか、プログラムが長いわけじゃけどこれ関数化すると分かりやすいなぁとか。
そういえば、プログラム中に出てくる直値も要注意です。
nest = 3;とか入れ子が3の深さという意味でしょうか?それともlevelが深さでしょうか?
だとするとnestに代入している数値の意味は?
enum定義できるならenumすべきだと思います。
Re: 綺麗なソースの書き方
Posted: 2012年1月14日(土) 02:57
by lriki
行間のとり方を意識してみると少しだけ見やすくなるかもしれません。
どうしてもひとつの関数に長いプログラムを書かないとならないときは
特に意識した方がいいと思います。
コード:
int revel=0;
int nest=1;
vector<int>check_same_nest;
for(int i=0;i<n;i++){
if(num1[ num2[i] ]<-1){
↓以下のような感じに。
コード:
int revel=0;
int nest=1;
vector<int>check_same_nest;
for(int i=0;i<n;i++){
if(num1[ num2[i] ]<-1){
あと、コメントもちょっと雑なところがあるかなと思いました。
コード:
now.nest=nest; //ネストを代入
例えばこの部分は nest の値を代入していることはコメントを見なくてもわかるわけですから、
「後で~に使うため、今回の nest の値を覚えておく」みたいな感じにできるとよいのではないでしょうか。
Cr さんが書きました:それと、なかなか良い変数の名前が浮かばないとき、どうしてますか?
どうしても思い浮かばなければコメントで変数の意味を説明しておくべきだと思います。
あとは翻訳サイトを活用してみるのもよいと思います。
Cr さんが書きました:コメントはどれぐらい書きますか?
私のプログラムは1/4くらいはコメントですね。
1行にひとつコメントを書くような書き方ではなく、クラスや関数の説明を詳細に書くようにしています。
あと、どんなにめんどくさいときでも関数ひとつにつき1行は必ずコメントを残してます。
個人的な開発でも、(3日前の自分はすでに他人ですので・・・)未来の自分に伝えるためにも重要です。
Re: 綺麗なソースの書き方
Posted: 2012年1月14日(土) 08:23
by 史上最悪のデスペナ
softya(ソフト屋) さんが書きました:書く人によって様々なスタイルがありますが私も変数宣言後にインデントするのは避けたいですね。
かくいう私も変数宣言後にインデントなんて
見たことないですが^^;
ふとある日
コード:
int a,b,c;
a = 4;
b = 5;
c = 6;
より
コード:
int a,b,c;
a = 4;
b = 5;
c = 6;
の方が見やすいんじゃね?俺天才!と思ってしまったのが運の付き・・・・・・・・
Re: 綺麗なソースの書き方
Posted: 2012年1月14日(土) 08:40
by beatle
そうそう,なんで史上最悪のデスペナさんは変数の初期化ではなく代入をするんでしょうか.
コード:
int a = 4, b = 5, c = 6;
とすれば良いと思います.
Cの世界では初期化でも代入でもほとんど同じだったのですが,C++の世界では初期化と代入では,効率がかなり違う場合がありますから注意しましょう.
効率が違うのは,クラスにはコンストラクタというものがあり,初期化時に呼び出されるからです.
コード:
class A; // なんらかのクラスA
A var1; // 変数定義(引数なしコンストラクタが呼び出される)
var1 = A(hoge); // 変数への代入(引数付きコンストラクタと代入演算子が呼び出される)
A var2(hoge); // 変数定義+初期化(引数付きコンストラクタが呼び出される)
また,const変数は初期化時以外に値を設定できませんので,否が応でも初期化する必要があります.
コード:
const int MAX_LENGTH1; // ERROR
const int MAX_LENGTH2 = 1024; // OK
Re: 綺麗なソースの書き方
Posted: 2012年1月14日(土) 08:44
by へろりくしょん
アルゴリズムについてはざっくり割愛しますが。
処理や機能としてまとめられる部分は、できるだけまとめてしまうと見やすいです。
たとえば、今回の main() 関数に関しても、55行目までを、変数の宣言・入力・初期化の3つに切り分ける事ができますね。
データの入力が行われる時には、入れ物はすべてそろっている状態。
初期化を行う時は、必要なデータはすべてそろっている状態。
これを意識するだけで、可読性はずっと上がります。 また、よりセキュアになりますし、デバッグも容易になります。
今回はそうでもありませんが、宣言・入力・初期化を各変数に対して細々とやったりすると、処理を追うのも大変なのはわかりますね。
どこで異常値が紛れてるのか、探すのも面倒ですし、エラーコードも書きにくいです。
後は、ブロックにはできるだけ明確な意味を持たせ、エラー処理等はできるだけ前に持ってくるのが定石です。
今回の例だと、solve() 関数、80行目から始まるループの本義は83行目の if() 文の中身です。
122行目の else 文は、ちょっと乱暴ですがエラー処理に相当しますね。
この場合は、83行目の if(num1[ num2
]<-1) が偽の場合、ループの意に沿わないとして、最初にトラップしておきます。
コード:
for(int i=0;i<n;i++){
if(num1[ num2[i] ] >= -1){ //返り点が連続しなかったとき
check_same_nest.clear(); //データをそれぞれ初期化
revel=0;
nest=1;
continue;
}
//ここから本当にやりたいこと。
・
・
・
}
こんな感じ。
関数は基本的に、上から順番に処理していく。
エラー等の場合は、途中退場。
成功時の途中退場は認めない。
そんで、エラー等は早めにトラップする。 ていうか、実際の処理を始める時にはデータに不整合等が無い事を保証する。
これは、最初に言った変数の宣言・入力・初期化の3つに切り分ける事も含みます。
と言っても、そうそう都合良く行かないのが世の中ですが、こういうスタイルを一貫して持つことは、
コードの可読性に大きく影響しますよ。
Re: 綺麗なソースの書き方
Posted: 2012年1月14日(土) 09:05
by 史上最悪のデスペナ
beatle さんが書きました:そうそう,なんで史上最悪のデスペナさんは変数の初期化ではなく代入をするんでしょうか.
コード:
int a = 4, b = 5, c = 6;
とすれば良いと思います.
なるほど。確かにそうすればいいですね(そういえば記憶の片隅にそんなものを見たことあるような気が)。
さらにそれを
コード:
int a = 4,
b = 5,
c = 6;
みたいにしてもいいかもしれませんね。今回の場合はやらなくてぱっと分かりますが。
Re: 綺麗なソースの書き方
Posted: 2012年1月14日(土) 09:11
by たかぎ
史上最悪のデスペナ さんが書きました:さらにそれを
コード:
int a = 4,
b = 5,
c = 6;
みたいにしてもいいかもしれませんね。今回の場合はやらなくてぱっと分かりますが。
そんな書き方をするぐらいなら...
コード:
int a = 4;
int b = 5;
int c = 6;
とする方が見やすいですし、書くのも楽です。
Re: 綺麗なソースの書き方
Posted: 2012年1月14日(土) 11:05
by 史上最悪のデスペナ
たかぎ さんが書きました:
そんな書き方をするぐらいなら...
コード:
int a = 4;
int b = 5;
int c = 6;
とする方が見やすいですし、書くのも楽です。
ああいう風にすればいいと思ったのは、
VC++だと改行で自動的にインデントが入りますし(まあ、4文字インデントにしてるためcharとかだとずれますが^^;)、もし仮に
コード:
int a = 4;
int b = 5;
int c = 6;
char d = 'A';
char e = 'B';
char f = 'C';
float g = 4.0f;
float h = 5.0f;
float i = 6.0f;
とかなってると
コード:
int a = 4,
b = 5,
c = 6;
char d = 'A',
e = 'B',
f = 'C';
float g = 4.0f,
h = 5.0f,
i = 6.0f;
の方が少しは見やすくなるかなと思ったのです。
Re: 綺麗なソースの書き方
Posted: 2012年1月14日(土) 11:12
by softya(ソフト屋)
コード:
int a = 4,
b = 5,
c = 6;
char d = 'A',
e = 'B',
f = 'C';
float g = 4.0f,
h = 5.0f,
i = 6.0f;
変数を減らすときに面倒なので私は使わないと思います。
見やすいかどうかは人次第と言った所でしょうか。
Re: 綺麗なソースの書き方
Posted: 2012年1月14日(土) 11:18
by 史上最悪のデスペナ
softya(ソフト屋) さんが書きました:変数を減らすときに面倒なので私は使わないと思います。
見やすいかどうかは人次第と言った所でしょうか。
確かにそうですよね。真ん中だけ消そうと思ったときにあの方法だとめんどくさいですもんね。
それに、もうある程度までいっちゃうと見やすいかどうかは人それぞれになってしまいますし。
Re: 綺麗なソースの書き方
Posted: 2012年1月14日(土) 14:51
by Cr
モノスゴイ数の返信があってびっくりしました。
みなさん、どうもありがとうございます。
>softya(ソフト屋)さん
softya(ソフト屋) さんが書きました:
何の12点なのかという視点が抜けています。
point12でもTwelvePointでも良いのですが更に何のと言う意味が欲しいです。
漢文のレ点、一二点、上下点、甲乙点とかの一二点でして
返り点の一二点だから、return_point_itiniになるのかなぁ?
伝わってないっていうことは分かりづらい変数名っていうことですよね…
こういうところにこそコメントを書くべきですかね?
softya(ソフト屋) さんが書きました:
添え字:場所 値:次に飛ぶ場所-今の場所 PointToNextPoint
添え字:場所 値:何回目に読むか PointToReadOrder
添え字:何回目に読むか 値:場所 ReadOrderToPoint
でしょうか。
添え字To値ですか
なるほど、わかりやすいですね
全部をひとまとめにして1つの名前を考えようとしているからだめだったのか…
softya(ソフト屋) さんが書きました:
蓄える物次第ですが無いより有ったほうが分かりやすいなら付けるべきです。
PointToNextPoint、 PointToReadOrder、ReadOrderToPointと付けるなら不要だと思います。
ただのnum1では直感的にintだとは思うでしょうが配列だとは思いません。
Toがついてたらvectorってわかりますもんね
num1はintか…
極力To使うようにして、使わなくても分かりやすい名前が浮かんだ時はv_つけるようにしてみます。
softya(ソフト屋) さんが書きました:
作っている最中に関数化や読みやすくするにはどうするかを常に考えています。
それでもやはり読みづらい物が出来る場合もあるので、それは後で見なおしてリファクタリングします。
とりあえず関数が30~40行を超えそうならヤバイかなぁとか、同じような処理が出てきたら関数化しようとか、プログラムが長いわけじゃけどこれ関数化すると分かりやすいなぁとか。
やっぱり関数化はちょこちょこするべきなんですね
長い、重複、複雑になってきたら関数化っと
softya(ソフト屋) さんが書きました:
そういえば、プログラム中に出てくる直値も要注意です。
enum定義できるならenumすべきだと思います。
ここでこそenumか
入門書で読んで知ってはいたのですが使ったことは一度もなかったですね
使う機会なんかないだろうって切り捨ててました
>梨樹さん
梨樹 さんが書きました:
行間のとり方を意識してみると少しだけ見やすくなるかもしれません。
↓以下のような感じに。
コード:
int revel=0;
int nest=1;
vector<int>check_same_nest;
for(int i=0;i<n;i++){
if(num1[ num2[i] ]<-1){
長く続いてたので改行せねばと思ってforで改行したのですが
forの後で改行するより前で改行したほうが読みやすいですね
まとまりは分割せずに改行すべきですね
梨樹 さんが書きました:
例えばこの部分は nest の値を代入していることはコメントを見なくてもわかるわけですから、
「後で~に使うため、今回の nest の値を覚えておく」みたいな感じにできるとよいのではないでしょうか。
確かに
コード:
now.nest=nest; //前回のネストを引き継ぎ
こうかな? 微妙…?
上中と返ってきて更に下へ返るとき、一二三の三にならないためにってことなんですが
梨樹 さんが書きました:
どうしても思い浮かばなければコメントで変数の意味を説明しておくべきだと思います。
あとは翻訳サイトを活用してみるのもよいと思います。
やはりコメントですか
翻訳サイトはもう点でした 活用したいと思います
梨樹 さんが書きました:
私のプログラムは1/4くらいはコメントですね。
1行にひとつコメントを書くような書き方ではなく、クラスや関数の説明を詳細に書くようにしています。
うーむ、どうやって書いたらクラスや関数の説明を詳細に書けるのかが分からない
例えば
コード:
int Total(int start, int end){ //総和を計算(end>=start)
int result = 0;
result = (start + end) * (end - start + 1) / 2; //等差数列の和の公式を利用
return result;
}
みたいに、最初に何をする関数なのか簡単に書いて、計算方法はその都度書く というやり方しか浮かびません
最初に詰め込むとむしろ分かりづらくなるような気がするのですが…
梨樹 さんが書きました:
あと、どんなにめんどくさいときでも関数ひとつにつき1行は必ずコメントを残してます。
関数一つに必ず一行ですか
その場合、プロトタイプ宣言の方にコメントを入れるのか、実際に関数の中身を書いてる所の先頭に入れるのか、どっちにしてますか?
梨樹 さんが書きました:
個人的な開発でも、(3日前の自分はすでに他人ですので・・・)未来の自分に伝えるためにも重要です。
それは実体験としてひしひしと感じました
テスト週間挟んで2週間も間が挟んだ時にコメント0のソースコード見たときの気分ったらもう…
書かないなんて後からみたら解読の時間の無駄にしかなりませんもんね
失礼ながら、勝手な時間の都合があり残りの方はまた返信させていただきます。
Re: 綺麗なソースの書き方
Posted: 2012年1月14日(土) 15:22
by beatle
直値(即値という方が僕はよく聞く)をenumで定義するのは,意味的におかしいので,特にC++ではやめたほうがいいんじゃないでしょうか.
C++ならconst変数を使って即値を定義したほうが良いと思います.
enum型と整数型が暗黙に相互変換できてしまう仕様は悪だ,と考えている人もいるくらいです.
Cでは
コード:
#define MAX_LINE 1024
C++では
コード:
const int MAX_LINE = 1024;
Re: 綺麗なソースの書き方
Posted: 2012年1月14日(土) 15:36
by softya(ソフト屋)
確かにintだとconstの方が良いですね。MAX_LINEはconstだと思います。
ただ、この場合enum 名前 {状態定義};の名前 nest;が良い気もしますがアルゴリズムをちゃんと確認していないので違うかも知れません。nestって状態じゃないんででしょうか?
Re: 綺麗なソースの書き方
Posted: 2012年1月16日(月) 21:08
by Cr
返事遅れてすみません
ちょっとここ数日あわただしくて
どうも、史上最悪のデスペナさんが話を広げてくださっていたようで^^
>史上最悪のデスペナさん
>たかぎさん
>beatleさん
>softya(ソフト屋)さん
勝手にまとめさせていただくと、
初期化は宣言と同時に行った方が処理的に早いことがあるし、すっきりする
『,』で区切っていくつかの変数を縦に並べて宣言する派 よりは
型を毎回書いて『;』で締めて縦に並べて宣言する派 の方が多い
っていう感じですかね?
>へろりあさん
へろりあ さんが書きました:
処理や機能としてまとめられる部分は、できるだけまとめてしまうと見やすいです。
たとえば、今回の main() 関数に関しても、55行目までを、変数の宣言・入力・初期化の3つに切り分ける事ができますね。
データの入力が行われる時には、入れ物はすべてそろっている状態。
初期化を行う時は、必要なデータはすべてそろっている状態。
これを意識するだけで、可読性はずっと上がります。 また、よりセキュアになりますし、デバッグも容易になります。
なるほど わける方法は関数化以外にもあるわけですね
確かに変数宣言と入力なんかは新しい入力が出てきたらその場で変数を作るなど結構行き当たりばったりでやってました。
読みやすいのは分かるのですが安全性も上がるんですねぇ
へろりあ さんが書きました:
コード:
for(int i=0;i<n;i++){
if(num1[ num2[i] ] >= -1){ //返り点が連続しなかったとき
check_same_nest.clear(); //データをそれぞれ初期化
revel=0;
nest=1;
continue;
}
//ここから本当にやりたいこと。
・
・
・
}
こんな感じ。
関数は基本的に、上から順番に処理していく。
エラー等の場合は、途中退場。
成功時の途中退場は認めない。
そんで、エラー等は早めにトラップする。 ていうか、実際の処理を始める時にはデータに不整合等が無い事を保証する。
これは、最初に言った変数の宣言・入力・初期化の3つに切り分ける事も含みます。
こっちの書きかただと本当にやりたいところのifのネスト一個少なくて済みますもんね
不正なデータは途中退場、心がけてみます。
へろりあ さんが書きました:
と言っても、そうそう都合良く行かないのが世の中ですが、こういうスタイルを一貫して持つことは、
コードの可読性に大きく影響しますよ。
そうですね トピックあげてから余計実感しました。
一回、今回のプログラムをみなさんの助言を元によりきれいに書き換えてみたいと思ってます。
手を動かせという意見は良く見ますし、正直忘れっぽいので慣れさせることにします。
返信一端ここで区切らせていただきます。
beatleさん、softya(ソフト屋)さんへの返信は次の投稿で
Re: 綺麗なソースの書き方
Posted: 2012年1月16日(月) 21:24
by Cr
>beatleさん
>softya(ソフト屋)さん
beatle さんが書きました:
直値(即値という方が僕はよく聞く)をenumで定義するのは,意味的におかしいので,特にC++ではやめたほうがいいんじゃないでしょうか.
softya(ソフト屋) さんが書きました:
この場合enum 名前 {状態定義};の名前 nest;が良い気もしますがアルゴリズムをちゃんと確認していないので違うかも知れません。nestって状態じゃないんででしょうか?
たぶん状態だと思います。
数字が何であってもかまわない、それを別の数字に置いても問題なく、その数字に割り当てられた意味が重要な時にenumを使うみたいな説明が入門書に書いてあったような気がします。
今回の場合はnestに格納されているのは一二点なのか、上下点なのか、甲乙点なのかという意味だと思います。
数字に変換するなら一二点系統の返り点に対し間にある一二点の数ということになるんですかね?
でもmoji[0][]を上中下点として利用するという特殊な使い方をしているので必ずしもそうではないです。
一二点などは『一』 『二』とついたら『二』はもう固定です
しかし上下点では『上』 『下』とついても、もう一つ出てきたら『上』 『中』 『下』(←こいつが増える)となり『下』→『中』という変化が起きます。
なので一二点をmoji[1][]に相当させ、上下点はmoji[2][]、上中下点をmoji[0][]としています
本来は上下点と上中下点は同じものなのですが、この方が楽なのでプログラム上では区別してあります
となると、やっぱり状態の意味合いが大きいように感じます。
Re: 綺麗なソースの書き方
Posted: 2012年1月16日(月) 21:34
by 史上最悪のデスペナ
Cr さんが書きました:どうも、史上最悪のデスペナさんが話を広げてくださっていたようで^^
私も気になる内容だったので便乗させてもらいましたw。お役に立てたなら良かったです。
Cr さんが書きました:自分で書いたのを後で見て何やってるか分かんないことがしばしばあります
そういえば、ふと思い出しましたが過去のトピックで
【雑談?】他人のコードを読み解くコツとは?poppin'さんがリンク先のようにおっしゃってました。
まあ、その下でsoftya(ソフト屋)さんが正反対のことをおっしゃってるのですが^^;(SEかゲームプログラマの違いによるもののようですが)
単純にぱっと見るだけなら仕様書を書いて同じファイルに一緒に保存しておくというのもありじゃないでしょうか?
トピックの趣旨とは若干(?)違いますが^^;
Re: 綺麗なソースの書き方
Posted: 2012年1月16日(月) 22:01
by softya(ソフト屋)
仕様書を別に書くと探したり・メンテナンスを忘れたりして大変です。
それならクラスや関数の仕様が十数行のコメントに渡っていても良いのでソースコードの関数の上にでも書かれていたほうが探す手間が省けます。
あとdoxygenなどを利用してドキュメントを生成する方法もあります。
「Doxygen」 ← ソースコードに特定の書き方をすることでドキュメントをソースコードから取り出す。
http://www.doxygen.jp/
公開するライブラリのマニュアルを作るときなどは便利です。
Re: 綺麗なソースの書き方
Posted: 2012年1月16日(月) 23:46
by Cr
言われたところをとりあえず全て直してみました
変数の名前、関数の名前、クラス、typedef語の名前などはbeatleさんが教えてくださったhttp://
www.textdrop.net/google-stylegui ... pguide.xml
を参考にさせてもらいました。
vector関連の変数名はsoftya(ソフト屋)さんからもらおうと思ったのですが、元read_orderが一度も計算に出てこないのでプログラムから完璧に削除し、その分区別が楽になったのでもう少し短い名前をつけてあります。
思ったことは変数名が長いとそれはそれで読みづらい気がするのですが、短くしようとして意味が伝わらなくなるのも問題でどこで折り合いをつけるべきなのか分からないということです。
皆さんはどうしてますか?
コード:
#include <iostream>
#include <iomanip>
#include <vector>
#include <string>
using namespace std;
string moji[8][10]={{"上","中","下"},
{"一","二","三","四","五","六","七"},
{"上","下"},
{"甲","乙","丙","丁","戊","己","庚","辛","任","癸"},
{"天","地","人"},
{"元","亨","利","貞"},
{"春","夏","秋","冬"},
{"木","火","土","金","水"}
};
struct OneTwo
{
int level; //一、二、三の違い
int nest; //一、上、甲の違い
};
typedef vector<int> VecInt;
typedef vector<OneTwo> VecOneTwo;
void solve(int n ,VecInt turn_to_point ,VecInt next_point ,VecOneTwo *edge);
void PrintVector(VecInt vec);
void SetReadOrderToPoint(VecInt *turn_to_point ,VecInt next_point ,int n);
int main(){
//宣言
VecInt turn_to_point; //添え字:読む順番 値:場所
VecInt next_point; //添え字:場所 値:次の場所
int n; //漢文の文字数
VecOneTwo edge; //添え字:場所 値:一二点系の返り点の種類
//入力
{
cout <<"漢文の文字数を入力"<<endl;
cin >> n;
next_point.resize(n);
cout << "漢文の読む順番を半角スペースで区切って入力"<<endl;
for (int i = 0; i < n; i++)
{
int a;
cin >> a;
next_point[a-1] = i;
}
}
//初期化
{
for (int i = 0; i < n; i++){
OneTwo start;
start.level = -1;
start.nest = -1;
edge.push_back(start);
}
SetReadOrderToPoint(&turn_to_point, next_point, n);
}
solve(n ,turn_to_point ,next_point ,&edge);//計算
//表示
{
for (int i = 0; i < n; i++)
{
//レ点
if (i != n && turn_to_point[i+1] == -1) //次に進む場所が一つ前なら
cout <<"レ";
else
cout <<" ";
//一二点系返り点
if (edge[i].nest != -1) //nestが初期値と違ったら
{
cout << moji[ edge[i].nest ][ edge[i].level ]; //一二点系返り点表示
if (turn_to_point[i] == 1) //一二点の後、行き先が一つ下なら
cout <<"|"; //ハイフン表示
else
cout <<" ";
}
else
{
cout <<" ";
}
cout<<":"<<endl;
}
}
return 0;
}
void solve(int n ,VecInt turn_to_point,VecInt next_point,VecOneTwo *edge){
//初期化
int level = 0;
int nest = 1;
VecInt check_same_nest;
for (int i = 0; i < n; i++)
{
if (turn_to_point[ next_point[i] ] >= -1)//返り点が連続しなかったら
{
check_same_nest.clear(); //データオール初期化
level = 0;
nest = 1;
continue;
}
//一、上などだけがここから下の処理を受ける
check_same_nest.push_back( next_point[i] ); //通った点を記録
for (int now = next_point[i]-1; now > next_point[i+1]; now--) //返り点の間に別の返り点があるかチェック
{
if ( (*edge)[now].nest >= nest) //自分以上にnestの高いのがいれば
{
nest = (*edge)[now].nest + 1; //それ+1を代入
}
}
if ( (*edge)[ check_same_nest[0] ].nest < nest) //昔のネストが今より低かった時
{
for (int i = 0; i < check_same_nest.size(); i++)
{
(*edge)[ check_same_nest[i] ].nest = nest; //過去の地点に今のネストを代入
}
}
//上下点処理
{
if (nest == 2)
{
switch (level)
{
case 1: //返り点が計3つになったら
nest = 0; //上中下点へ(上点は変わらないので無視)
break;
case 2: //返り点が計4つになったら
nest = 3; //過去のもさかのぼって甲乙点へ
for (int i = 0; i < 2; i++)
{
(*edge)[ check_same_nest[i] ].nest=3;
}
break;
}
}
}
OneTwo now;
now.level = level; //level引き継ぎ
now.nest = nest; //nestを引き継ぎ
(*edge)[ next_point[i] ] = now; //答えに代入
now.level++;
(*edge)[ next_point[i+1] ] = now; //返る場所にも返り点をつける ex:二,下
}
}
void SetReadOrderToPoint(VecInt *turn_to_point, VecInt next_point, int n)//read_order_to_pointを初期化
{
(*turn_to_point).resize(n);
for (int i = 0; i < n - 1; i++)
{
(*turn_to_point)[ next_point[i] ] = next_point[i+1] - next_point[i];//相対距離を代入
}
(*turn_to_point)[ next_point[n-1] ] = 0;//終点には0を代入
}
void PrintVector(VecInt vec) //与えられた配列の全要素を表示
{
for(int i = 0; i < vec.size(); i++)cout <<setw(2) <<internal <<vec[i] <<endl;
cout <<endl;
}
もとよりは分かりやすくなりましたかね?^^;
ダメ出しなど、またあればお願いします。
Re: 綺麗なソースの書き方
Posted: 2012年1月17日(火) 12:34
by softya(ソフト屋)
気になる所は●印でコメントが書いてあります。
一部を直しましたが全部は見直していません。
コード:
// ●何をするプログラムか書いて欲しい。
#include <iostream>
#include <iomanip>
#include <vector>
#include <string>
using namespace std;
// ●どういう働きをするかコメント
// ●下記のネストとレベルの関係など
// ●contで8と10に名前をつけたほうが良いでしょう。
string moji[8][10]={{"上","中","下"},
{"一","二","三","四","五","六","七"},
{"上","下"},
{"甲","乙","丙","丁","戊","己","庚","辛","任","癸"},
{"天","地","人"},
{"元","亨","利","貞"},
{"春","夏","秋","冬"},
{"木","火","土","金","水"}
};
// ●OneTwoで一二点処理と言いながら上下点も甲乙点処理もしています。
// ●返り点の情報なのでは? という意味では名前の付け方が値の本質からずれています。
struct OneTwo
{
int level; //一、二、三の違い
int nest; //一、上、甲の違い
};
typedef vector<int> VecInt;
typedef vector<OneTwo> VecOneTwo;
// ●ポインタ渡しから参照に書き換えました。
// ●TurnToPointは名詞to名詞の形になっていないので出来れば避けてください。
// ●ReadOrderToPointは無いのでTurnToPointに統一。
void SetTurnToPoint(VecInt &turn_to_point ,VecInt next_point ,int mojilen); // 返り点情報を初期化
void solve(int mojilen ,VecInt turn_to_point ,VecInt next_point ,VecOneTwo &edge); // 計算
void PrintVector(VecInt vec);
int main(){
//宣言 ●コメント修正。絶対位置と相対位置など明確に。場所とか順番とか違うように見える部分も修正。
// ●turn_to_pointではなくNextPointToOffsetでは無いでしょうか?
VecInt turn_to_point; //添え字:次の場所 値:次の場所の相対位置
VecInt next_point; //添え字:場所 値:次の場所
int mojilen; //漢文の文字数 ●わかり易い名前
VecOneTwo edge; //添え字:場所 値:一二点系の返り点の種類
//入力
{
cout <<"漢文の文字数を入力"<<endl;
cin >> mojilen;
next_point.resize(mojilen);
cout << "漢文の読む順番を半角スペースで区切って入力"<<endl;
for (int i = 0; i < mojilen; i++)
{
int a;
cin >> a;
next_point[a-1] = i;
}
}
//初期化
{
for (int i = 0; i < mojilen; i++){
OneTwo start;
start.level = -1;
start.nest = -1;
edge.push_back(start);
}
SetTurnToPoint(turn_to_point, next_point, mojilen);
}
//計算
solve(mojilen ,turn_to_point ,next_point ,&edge);
//表示
{
for (int i = 0; i < mojilen; i++) // ●{}の位置は統一しましょう。
{
//レ点
if (i != mojilen && turn_to_point[i+1] == -1) //次に進む場所が一つ前なら
cout <<"レ";
else
cout <<" ";
//一二点系返り点
if (edge[i].nest != -1) //nestが初期値と違ったら
{
cout << moji[ edge[i].nest ][ edge[i].level ]; //一二点系返り点表示
if (turn_to_point[i] == 1) //一二点の後、行き先が一つ下なら
cout <<"|"; //ハイフン表示
else
cout <<" ";
}
else
{
cout <<" ";
}
cout<<":"<<endl;
}
}
return 0;
}
void solve(int mojilen ,VecInt turn_to_point,VecInt next_point,VecOneTwo &edge)
{
//初期化
int level = 0;
int nest = 1;
VecInt check_same_nest;
for (int i = 0; i < mojilen; i++)
{
if (turn_to_point[ next_point[i] ] >= -1)//返り点が連続しなかったら
{
check_same_nest.clear(); //データオール初期化
level = 0;
nest = 1;
continue;
}
//一、上などだけがここから下の処理を受ける
check_same_nest.push_back( next_point[i] ); //通った点を記録
for (int now = next_point[i]-1; now > next_point[i+1]; now--) //返り点の間に別の返り点があるかチェック
{
if ( edge[now].nest >= nest) //自分以上にnestの高いのがいれば
{
nest = edge[now].nest + 1; //それ+1を代入
}
}
if ( edge[ check_same_nest[0] ].nest < nest) //昔のネストが今より低かった時
{
for (int i = 0; i < check_same_nest.size(); i++)
{
edge[ check_same_nest[i] ].nest = nest; //過去の地点に今のネストを代入
}
}
//上下点処理
{
if (nest == 2)
{
switch (level)
{
case 1: //返り点が計3つになったら
nest = 0; //上中下点へ(上点は変わらないので無視)
break;
case 2: //返り点が計4つになったら
nest = 3; //過去のもさかのぼって甲乙点へ
for (int i = 0; i < 2; i++)
{
edge[ check_same_nest[i] ].nest=3;
}
break;
}
}
}
OneTwo now;
now.level = level; //level引き継ぎ
now.nest = nest; //nestを引き継ぎ
edge[ next_point[i] ] = now; //答えに代入
now.level++;
edge[ next_point[i+1] ] = now; //返る場所にも返り点をつける ex:二,下
}
}
// 返り点情報を初期化
void SetTurnToPoint(VecInt &turn_to_point, VecInt next_point, int mojilen)
{
turn_to_point.resize(mojilen);
for (int i = 0; i < mojilen - 1; i++)
{
turn_to_point[ next_point[i] ] = next_point[i+1] - next_point[i];//相対距離を代入
}
turn_to_point[ next_point[mojilen-1] ] = 0;//終点には0を代入
}
// 与えられた配列の全要素を表示
void PrintVector(VecInt vec)
{
for(int i = 0; i < vec.size(); i++)cout <<setw(2) <<internal <<vec[i] <<endl;
cout <<endl;
}