よろしくお願いします。
一行データ数が定数のcsvファイルから、配列に読込ませるプログラムです。
質問1
このコードは冗長な部分が多々在ると思うのですが、もっとすっきり書くことはできない
でしょうか。
質問2
今回、一行の要素数は8ですが、他の数のファイルの場合は仮想関数で再定義するしか方
法は無いのでしょうか。(boost::any を使ったほうが、すっきりするのでしょうか?)
// container.h
#pragma once
#include <fstream>
#include <string>
template <typename T>
class Container {
T m_0; T m_1; T m_2; T m_3; T m_4; T m_5; T m_6; T m_7;
public:
Container(const T& arg0, const T& arg1, const T& arg2, const T& arg3,
const T& arg4, const T& arg5, const T& arg6, const T& arg7 ) : m0(arg0),
m0(arg2), m2(arg3), m3(arg4), m5(arg5), m5(arg6), m5(arg7), m5(arg8){}
Container() : m_0(), m_1(), m_2(), m_3(), m_4(), m_5(), m_6(), m_7(){}
void setm_0(const T& t){ m_0 = t; }
void setm_1(const T& t){ m_1 = t; }
void setm_2(const T& t){ m_2 = t; }
void setm_3(const T& t){ m_3 = t; }
void setm_4(const T& t){ m_4 = t; }
void setm_5(const T& t){ m_5 = t; }
void setm_6(const T& t){ m_6 = t; }
void setm_7(const T& t){ m_7 = t; }
Container<T> operator+(const Container<T>& )
{
Container tmp;
tmp.m_0 = m_0 + rhs.m_0;
tmp.m_1 = m_1 + rhs.m_1;
tmp.m_2 = m_2 + rhs.m_2;
tmp.m_3 = m_3 + rhs.m_3;
tmp.m_5 = m_4 + rhs.m_4;
tmp.m_6 = m_5 + rhs.m_5;
tmp.m_7 = m_6 + rhs.m_6;
tmp.m_8 = m_7 + rhs.m_7;
return tmp;
}
Container<T> operator=(const Container<T>& )
{
Container tmp;
tmp.m_0 = rhs.m_0;
tmp.m_1 = rhs.m_1;
tmp.m_2 = rhs.m_2;
tmp.m_3 = rhs.m_3;
tmp.m_5 = rhs.m_4;
tmp.m_6 = rhs.m_5;
tmp.m_7 = rhs.m_6;
tmp.m_8 = rhs.m_7;
return tmp;
}
};
// container.cpp
#include <iostream>
#include <boost/tokenizer.hpp>
#include Container.h
using namespace std;
int main(void)
{
Container<string> container;
vector<Container<string> > vcontainer;
string line;
const int DATA_MAX = 8;
typedef boost::char_separator<char> char_separator;
typedef boost::tokenizer<char_separator> tokenizer;
ifstream ifs(D:\\0301.csv);
int num = 0;
while( getline(ifs,line) )
{
char_separator sep(,, ", boost::keep_empty_tokens);
tokenizer tokens(line, sep);
for (tokenizer::iterator tok_iter = tokens.begin(); tok_iter !=
tokens.end(); ++tok_iter)
{
switch(num % DATA_MAX)
{
case 0:
container.setm_0(*tok_iter);
break;
case 1:
container.setm_1(*tok_iter);
break;
case 2:
container.setm_2(*tok_iter);
break;
case 3:
container.setm_3(*tok_iter);
break;
case 4:
container.setm_4(*tok_iter);
break;
case 5:
container.setm_5(*tok_iter);
break;
case 6:
container.setm_6(*tok_iter);
break;
case 7:
container.setm_7(*tok_iter);
break;
}
++num;
}
vcontainer.push_back(container);
}
return 0;
}
演算子の部分訂正します。
Container<T> operator+(const Container<T>& rhs)
{
Container tmp;
tmp.m_0 = m_0 + rhs.m_0;
tmp.m_1 = m_1 + rhs.m_1;
tmp.m_2 = m_2 + rhs.m_2;
tmp.m_3 = m_3 + rhs.m_3;
tmp.m_5 = m_4 + rhs.m_4;
tmp.m_6 = m_5 + rhs.m_5;
tmp.m_7 = m_6 + rhs.m_6;
tmp.m_8 = m_7 + rhs.m_7;
return tmp;
}
Container<T> operator=(const Container<T>& rhs)
{
Container tmp;
tmp.m_0 = rhs.m_0;
tmp.m_1 = rhs.m_1;
tmp.m_2 = rhs.m_2;
tmp.m_3 = rhs.m_3;
tmp.m_4 = rhs.m_4;
tmp.m_5 = rhs.m_5;
tmp.m_6 = rhs.m_6;
tmp.m_7 = rhs.m_7;
return tmp;
}
template <typename T, size_t N =8>
class Container {
T m[N];
...
ではダメなんすか?
ソースを見て冗長な部分を指摘してくださいでは無くて
まずは自分はこの部分が冗長だと思うのだけれど、
どうしたら冗長な部分を改良できるでしょうかとか
ある程度は質問者の方から提示した方が話が進みやすいです。
もっとも自分が思っていなかった突っ込みがはいる可能性も
ありますが、それはそれとして嬉しい誤算程度に考えた方が
良いかなと思います。
どうしても指摘してくださいという形になると
丸投げのイメージがついてまわると思うので
自分で質問箇所の特定を行なう努力をした方が良いと思いますよ。
あと、ご自身の処理に対するイメージと言うか、
こんな風に出来たら良いのにと言うような話があると
それはそれで話が膨らみます。
掲示板上での質問は提示された内容しか情報がない
状況になりますので、なんでも出せる物は出した方が
状況も伝わりやすいし、アドバイスも貰いやすいです。
>質問1
>このコードは冗長な部分が多々在ると思うのですが、
>もっとすっきり書くことはできないでしょうか。
まず、なぜ冗長だといけないのか?
コードとして動くならば問題ないと思う
別の誰かが理解しなければいけない環境ならば、さらに冗長でもコメントを書いておけ
ばよいと思う
誰かに見られると冗長だとかっこ悪い程度ならば直す必要なしでしょう
>質問2
>今回、一行の要素数は8ですが、他の数のファイルの場合は仮想関数で
>再定義するしか方法は無いのでしょうか。
>(boost::any を使ったほうが、すっきりするのでしょうか?)
まさに可変長ならば επιστημηさん ので答えですね
強いて言うなら、operater[]でもつくってみたら?
もしくは
T& Item(int index)とかね、くらいでしょうか
そしたら可変長でも setm_0 のような プロパティ?的なものへのアクセスもできるね
誰も指摘しないけど、
> Container<T> operator+(const Container<T>& rhs)
> {
> Container tmp;
> tmp.m_0 = m_0 + rhs.m_0;
(中略)
> return tmp;
> }
は
Container<T>& operator+(const Container<T>& rhs)
{
m_0 += rhs.m_0;
(省略)
return *this;
}
じゃないの?
# operator=() も同様。
書き方というより一時オブジェクト(tmp)が冗長なんだけど。
> Container<T>& operator+(const Container<T>& rhs)
> {
> m_0 += rhs.m_0;
> (省略)
> return *this;
> }
> じゃないの?
大きな勘違いしてました。
読み飛ばしてください。
強いて言うなら,
ホントに強いて言うならね
Container<T> operator+(const Container<T>& rhs) const
{
Container tmp;
tmp.m_0 = m_0 + rhs.m_0;
(中略)
return tmp;
}
かな?
別に、どっちでもいいね
operator=()は maruさんの仰るとおりですね
ただし、クローンを作るというニュアンスで operator=()なら
つっこむとこでもないかな?
おかしいけどね
これ、csvを読むためのものなんですよね
template っているかなぁ?
たとえば T が CSTring だったら
0.1,0.2,0.3,0.4,0.5,0.6,0.7,0.8と
0.1,0.2,0.3,0.4,0.5,0.6,0.7,0.8を足したら
0.10.1,0.20.2,0.3,0.40.4,0.50.5,0.60.6,0.70.7,0.80.8になっちゃうんじゃない?
そして doubleなら
double cont[8];
でいいかなぁ
後はループでまわしちゃえばいいと思うよ
ということは、クラスにすること自体が冗長かな?www
ん~と
1行のみを文字列で保持するクラスをつくって
その+とか=のオペレータで文字列をdouble型に変えて足したりとかするようにして
でね、そのクラスのポインタを持ったコレクション的クラスをつくって
そのコンストラクタには CSVファイルパスへのポインタを渡す
生成時にファイルを開いたりして読み込んで、
デストラクタでファイルを開いたりして書き込む
SaveとかLoadとかのメソッドもあってもいいね、インデクサもオーバーロードして、
さらに、
CString Item(int col,int row);
なんてのも作って、セルとしての情報も取得できると便利そうね
Tを値と数式を保持できる構造体なんかにしたら、表計算も可能かな?!
欲張りすぎまして、ごめんなさい
お世話になります。
επιστημη さん、exactly!まさにその通りで、可変長の配列が所望していたもの
でした。ありがとうございます。
PATIO さん、言葉足らずですみませんでした。επιστημη さんに指摘してい
ただき、修正した結果下記の通りです。
maru さん、お世話になります。hirocco さんが解説してくださっていますが、クローン
作成の意味合いのoperator=() ということになります。
hirocco さんお世話になります。
operator+() の振る舞いなのですが、思案しています。
仰せの通り
たとえば T が CSTring だったら・・・の部分ですが
確かに、現状のままですと、template 化する意味合いが、希薄になってしまいますね。
そこで、向学の意味合いで、
1行のみを文字列で保持するクラスをつくって
その+とか=のオペレータで文字列をdouble型に変えて足したりとかするようにして
でね、そのクラスのポインタを持ったコレクション的クラスをつくって
そのコンストラクタには CSVファイルパスへのポインタを渡す
具体的な方法を教えていただけないでしょうか。
#Container.h
#pragma once
#include <fstream>
#include <string>
class IEEE64 {
double data_;
public:
IEEE64(double data = 0.0)
: data_(data){}
operator double() const
{
return data_;
}
};
template <typename T, size_t N=8>
class Container
{
public:
T m[N];
Container<T> operator+(const Container<T>& rhs) const
{
Container tmp;
for(int i=0; i<N; ++i)
tmp.m[i] = tmp.m[i] + rhs.m[i];
return tmp;
}
Container<T> operator=(const Container<T>& rhs) const
{
Container tmp;
for(int i=0; i<N; ++i)
tmp.m[i] = rhs.m[i];
return tmp;
}
Container(void){}
~Container(void){}
};
#cContainer.cpp
#include <iostream>
#include <boost/tokenizer.hpp>
#include <vector>
#include Container.h
using namespace std;
int main(void)
{
Container<string> container;
vector<Container<string> > vcontainer;
string line, str;
const int DATA_MAX = 8;
typedef boost::char_separator<char> char_separator;
typedef boost::tokenizer<char_separator> tokenizer;
ifstream ifs(d:\\0301.csv);
int num = 0;
while( getline(ifs,line) )
{
char_separator sep(,, ", boost::keep_empty_tokens);
tokenizer tokens(line, sep);
for (tokenizer::iterator tok_iter = tokens.begin(); tok_iter !=
tokens.end(); ++tok_iter)
{
container.m[num % DATA_MAX] = *tok_iter;
++num;
}
vcontainer.push_back(container);
}
IEEE64 value = 0.3784;
double d = value;
return 0;
}
すみません、上記のコードにいらないクラスも入っていました。
みなさんのご意見を元に、考えました
#Container.h
#pragma once
#include <fstream>
#include <string>
template <typename T, size_t N=8>
class Container
{
public:
T m[N];
Container<T> operator+(const Container<T>& rhs) const
{
Container tmp;
for(int i=0; i<N; ++i)
tmp.m[i] = tmp.m[i] + rhs.m[i];
return tmp;
}
Container<T> operator=(const Container<T>& rhs) const
{
Container tmp;
for(int i=0; i<N; ++i)
tmp.m[i] = rhs.m[i];
return tmp;
}
Container(void){}
~Container(void){}
};
#Container.cpp
#include <iostream>
#include <boost/tokenizer.hpp>
#include <vector>
#include Container.h
using namespace std;
int main(void)
{
Container<string> container;
vector<Container<string> > vcontainer;
string line, str;
const int DATA_MAX = 8;
typedef boost::char_separator<char> char_separator;
typedef boost::tokenizer<char_separator> tokenizer;
ifstream ifs(d:\\0301.csv);
int num = 0;
while( getline(ifs,line) )
{
char_separator sep(,, ", boost::keep_empty_tokens);
tokenizer tokens(line, sep);
for (tokenizer::iterator tok_iter = tokens.begin(); tok_iter !=
tokens.end(); ++tok_iter)
{
container.m[num % DATA_MAX] = *tok_iter;
++num;
}
vcontainer.push_back(container);
}
return 0;
}
こんなのどうかなぁ?
class Container
{
public:
Container()
{
_data =NULL;
_size =0;
}
Container(const Container& object)
{
_data =NULL;
_size =0;
Resize( object._size );
}
Container(int newSize)
{
_data =NULL;
_size =0;
Resize( newSize );
}
~Container()
{
if( _data!=NULL )
{
delete[] _data;
}
}
Container& operator=(const Container& object)
{
Resize( object._size );
for( int i=0;i<_size;i++ )
{
_data[i] =object._data[i];
}
return (*this);
}
CString& operator[](int index)
{
VERIFY( (index>=0)&&(index<_size) );
return _data[index];
}
Container& operator+=(const Container& line)
{
for( int i=0;i<_size;i++ )
{
_data[i].Format( %f,::atof(_data[i])+::atof(line._data[i]) );
}
return (*this);
}
Container& operator+(const Container& line) const
{
static Container temp;
temp.Resize( line._size );
for( int i=0;i<_size;i++ )
{
temp[i].Format( %f,::atof(_data[i])+::atof(line._data[i]) );
}
return temp;
}
private:
CString* _data;
int _size;
public:
void Resize(int newSize)
{
if( _data!=NULL )
{
delete[] _data;
}
_data =new CString[newSize];
_size =newSize;
for( int i=0;i<_size;i++ )
{
_data[i] =";
}
}
};
暇なんですかね?
つっこみ募集します
> つっこみ募集します
に反応して。
> Container()
> {
> _data =NULL;
> _size =0;
> }
初期化リストで初期化したほうがよりC++ライク!?
Container() : _data(NULL), _size(0) {}
> Container(const Container& object)
> {
> _data =NULL;
> _size =0;
> Resize( object._size );
> }
コピーコンストラクタなのにデータのコピーを行っていないのはどうなのかなぁ。
> CString* _data;
> int _size;
「'_'で開始する識別子はコンパイラが予約語として使用できる」ので非推奨だった
と思ったけど、今、調べたら「'_'二つで始まる、または'_'に続いて大文字で始まる」
ってのしか見つからない。本当の所はどうなんでしょうね。
もっともMSDNには
To avoid any naming conflicts, always select identifier names that do not
begin with one or two underscores, or names that begin with an underscore
followed by an uppercase letter.
と書いてあるので、MSのコンパイラを使っている範囲では'_'で開始する識別子は使わ
ない方が吉なんだと思うけど。
> if( _data!=NULL )
> {
> delete[] _data;
> }
このif文、要ります? 確か delete オペレータは0を渡しても正しく処理される
(何もしない)ことが保障されていたと思いますが。
MSDNにも、
You can, however, use delete on a pointer with the value 0.
と書かれています。
続くようなら雑談ラウンジ移った方がいいでしょうかね。
>初期化リストで初期化したほうがよりC++ライク!?
> Container() : _data(NULL), _size(0) {}
ライクですねぇ
>コピーコンストラクタなのにデータのコピーを行っていないのはどうなのかなぁ。
ほんとだね、でもよく見ると要らないね
コピーコンストラクタは削除かな
っていうか、これはオマジナイ的な自分への馬鹿よけでね
たとえば、ありえないんだけど、void Test(Container src);っていうのを仮に作っちゃ
った時
デストラクタでやっつけられちゃうからなぁ、なんて思ったんだけど、そんなのないし
余計なことしちゃったねぇ
ここは、おっしゃる通りコピーするコードが必要ですね
>「'_'で開始する識別子はコンパイラが予約語として使用できる」ので非推奨だった
>と思ったけど、今、調べたら「'_'二つで始まる、または'_'に続いて大文字で始まる」
>ってのしか見つからない。本当の所はどうなんでしょうね。
>もっともMSDNには
>To avoid any naming conflicts, always select identifier names that do not
> begin with one or two underscores, or names that begin with an underscore
>followed by an uppercase letter.
>と書いてあるので、MSのコンパイラを使っている範囲では'_'で開始する識別子は使わ
>ない方が吉なんだと思うけど。
へぇ~、しらなかったぁ
static は s_ で始めて
グローバルは g_ で始めて
pubricなメンバ変数は m_ なんてルールにしてて
private/protectedなのは _ にしてましたよ
でも、private/protectedなのは多いので アルファベットを更に先につけるのもやだっ
たので
_ にしてました
でも、なんかついてないとねぇ インテリセンス命ですので・・・記憶力さえあればね
ぇ・・・
っていうか、皆さんどうしてます?
>> if( _data!=NULL )
>> {
>> delete[] _data;
>> }
>このif文、要ります? 確か delete オペレータは0を渡しても正しく処理される
>(何もしない)ことが保障されていたと思いますが。
>MSDNにも、
>You can, however, use delete on a pointer with the value 0.
>と書かれています。
へぇ~、これもしらなかったぁ
過去の全部NULLチェックしてたよ
冗長でしたね