回線はどのように動作しますか？

Question

私は自分のgetline関数を書こうとしており、segfaultingを続けています。どのように私はそれを修正すると正式に鉱山が機能していない場合、ラインの仕事を得るのですか？私はコードを良くする方法を学ぶためにこれを書いています。

#include"MyString.h" 





MyString::MyString() //constructor 
{ 
    size=0; 
    capacity=1; 
    data=new char[capacity]; 

} 
MyString::MyString(char * n) //copy constructor 
{ 
    size=strlen(n); 
    capacity=strlen(n)+1; 
    data=new char[capacity]; 
    strcpy(data,n); 
} 
MyString::MyString(const MyString &right) // 
{ 
    size=strlen(right.data); 
    capacity=strlen(right.data)+1; 
    data=new char [capacity]; 
    strcpy(data,right.data); 

} 
MyString::~MyString() 
{ 
    delete [] data; 
} 
MyString MyString::operator = (const MyString& s) 
{ 

    if(this!=&s) 
    { 
     MyString temp=data; 
     delete [] data; 
     size=strlen(s.data); 
     capacity=size+1; 
     data= new char [capacity]; 
     strcpy(data,s.data); 
    } 
} 
MyString& MyString::append(const MyString& s) 
{ 
    if(this!=&s) 
    { 
     strcat(data,s.data); 
    } 


} 
MyString& MyString::erase() 
{ 

} 
MyString MyString::operator + (const MyString& s)const 
{ 
    return strcat(data,s.data); 
} 
bool MyString::operator == (const MyString& s) 
{ 
    return strcmp(data,s.data)==0; 
} 
bool MyString::operator < (const MyString& s) 
{ 
    return strcmp(data,s.data)<0; 
} 
bool MyString::operator > (const MyString& s) 
{ 
    return strcmp(data,s.data)>0; 
} 
bool MyString::operator <= (const MyString& s) 
{ 
    return strcmp(data,s.data)<=0; 
} 
bool MyString::operator >= (const MyString& s) 
{ 
    return strcmp(data,s.data)>=0; 
} 
bool MyString::operator != (const MyString& s) 
{ 
    return strcmp(data,s.data)!=0; 
} 
void MyString::operator += (const MyString& s) 
{ 
    append(s.data); 
} 
char& MyString::operator [ ] (int n) 
{ 
    return data[n]; 
} 
void MyString::getline(istream& in) 
{ 
    char c; 
    erase(); 
    ifstream input; 
    while(in.get(c)&&c!='\n') 
    { 
     data[size]=c; 
     size++; 

     if(size+1<=capacity) 
     { 
      capacity*=2; 
      char*p=new char[capacity]; 
      strcpy(p,data); 
      delete [] data; 
      data=p; 
     } 
     data[size]=c; 
     size++; 
     data[size]='\0'; 
    } 

} 
int MyString::length() const 
{ 
    return strlen(data); 
} 
void MyString::grow() 
{ 
capacity=strlen(data)+1; 
MyString temp; 
temp=data; 
delete [] data; 
capacity*=2; 
data= new char[capacity]; 
} 

ostream& operator<<(ostream& out, MyString& s) 
{ 

    out<<s.data; 
    return out; 


} 



// int MyString::getCapacity(){return capacity;} 

Answer 1

それは次のようになります。

void MyString::getline(istream& in) 
{ 
    erase(); 
    for (char c; in.get(c) && c != '\n';) 
     append(c); 
} 

は今、あなたはちょうど適切に単一の文字を追加しAPPENDというメソッドを実装する必要があります。それに問題がある場合は、別の質問をしてください。あなたは私がここで面白いと思うかもしれませんが、私はそうではありません。あなたは、あなたが再割り当てする場所を制限し、あなた自身のコードで自分自身の繰り返しを止める必要があります。 getline関数は、その種のアクティビティの場所ではありません（再割り当て、私は意味します）。

Answer 2

うーん...

if(size+1<=capacity) 

のは、あなたの能力が11であり、あなたのサイズがあなたがif(size >= capacity)をしたい11

if(12 <= 11) 
{ 
    // Resize capacity. This code won't run. 
} 

あるとしましょう。

また、data[size] = c; size++;がループ内に2回あります。だからあなたはすべてのキャラクターの2つのコピーを作っています。

Answer 3

strlenを2回呼び出す必要はありません。オプティマイザは、冗長性を排除するほどスマートではありません。また、渡された文字列のデータを変更していないので、constにする必要があります。それ以外の場合は、文字列定数を渡すことができませんでした。修正、我々が得る：

MyString::MyString(const char * n) //copy constructor 
{ 
    size = strlen(n); 
    capacity = size + 1; 
    data = new char[capacity]; 
    strcpy(data, n); 
} 

---

MyString::MyString(const MyString &right) // 
{ 
    size=strlen(right.data); 
    capacity=strlen(right.data)+1; 
    data=new char [capacity]; 
    strcpy(data,right.data); 

} 

rightはstrlenの必要性を排除しsize部材を有します。それはプラス上記の修正を与える：

MyString::MyString(const MyString &right) // 
{ 
    size = right.size; 
    capacity = size + 1; 
    data = new char[capacity]; 
    strcpy(data, right.data); 
} 

---

MyString MyString::operator = (const MyString& s) 
{ 

    if(this!=&s) 
    { 
     MyString temp=data; 
     delete [] data; 
     size=strlen(s.data); 
     capacity=size+1; 
     data= new char [capacity]; 
     strcpy(data,s.data); 
    } 
} 

operator =参照を返す必要があります;、ではない別のコピーを（すなわち*thisはところで、あなたは何を返すのを忘れ）。またtempが使用されていないことに注意してください、とあなたは冗長strlen電話を持っている：

MyString& MyString::operator = (const MyString& s) 
{ 
    if(this!=&s) 
    { 
     delete[] data; 
     size = s.size; 
     capacity = size + 1; 
     data = new char[capacity]; 
     strcpy(data, s.data); 
    } 
    return *this; 
} 

---

MyString& MyString::append(const MyString& s) 
{ 
    if(this!=&s) 
    { 
     strcat(data,s.data); 
    } 


} 

あなたがチェックするのを忘れて、あなたのsizeとcapacity。したがっては、char配列の最後ですぐに実行されます。また、自分自身への連結を働かせなければならない。そして、あなたはstrcatを必要としない：

MyString& MyString::append(const MyString& s) 
{ 
    size_t rSize = s.size; 
    if(capacity < size + rSize + 1) 
    { 
     capacity = size + rSize + 1; 
     char* newData = new char[capacity]; 
     memcpy(newData, data, size); 
     delete[] data; 
     data = newData; 
    } 
    memcpy(data + size, s.data, rSize); 
    size += rSize; 
    data[size] = '\0'; 
    return *this; 
} 

---

erase単なる文字列が離れて行くようにする必要があります。いいえメモリがでめちゃめちゃする必要はありません：appendと同様に

MyString& MyString::erase() 
{ 
    size = 0; 
    data[0] = '\0'; 
} 

---

MyString MyString::operator + (const MyString& s)const 
{ 
    return strcat(data,s.data); 
} 

同じ問題に加え、この1は、新しいオブジェクトを返すと、一人でその入力を残す必要があります。これは陰湿なものです

MyString MyString::operator + (const MyString& s) const 
{ 
    return MyString(*this).append(s); 
} 

---

void MyString::operator += (const MyString& s) 
{ 
    append(s.data); 
} 

：また、これまでに書かれている方法を再利用！ appendはconst MyString&なので、char*を与えると、コンパイラはMyStringというテンポラリを作成し、MyString(const char*)コンストラクタを呼び出し、テンポラリをappendに渡して破棄します。正しい結果が得られますが、余分なオブジェクトを作成しました。正しい方法は次のとおりです。

void MyString::operator += (const MyString& s) 
{ 
    append(s); 
} 

---

char& MyString::operator [ ] (int n) 
{ 
    return data[n]; 
} 

ませ境界チェックしませんか？勇敢。あなたがそれを望むなら、それを加えるのは難しくありません。あなたが例外をスローしたくないと仮定：

char& MyString::operator [] (int n) 
{ 
    if(n < 0) 
     n = 0; 
    if(n >= size) 
     n = size - 1; 
    return data[n]; 
} 

はFYI、インデックスの通常のタイプは<stddef.h>で定義されているsize_t、あるとsizeof()によって返された符号なし整数型であるので、それは次のように動作する保証です通常の配列を持つ配列インデックスです（ここでは、負のインデックスチェックを取り除くことができます）。

また、const MyStringで動作するバージョンが必要な場合もあります。それだけではなく、参照の文字を返します。

char MyString::operator [] (int n) const 
{ 
    if(n < 0) 
     n = 0; 
    if(n >= size) 
     n = size - 1; 
    return data[n]; 
} 

---

void MyString::getline(istream& in) 
{ 
    char c; 
    erase(); 
    ifstream input; 
    while(in.get(c)&&c!='\n') 
    { 
     data[size]=c; 
     size++; 

     if(size+1<=capacity) 
     { 
      capacity*=2; 
      char*p=new char[capacity]; 
      strcpy(p,data); 
      delete [] data; 
      data=p; 
     } 
     data[size]=c; 
     size++; 
     data[size]='\0'; 
    } 

} 

まず、ifstreamは何もしません。それを取り除く。 istreamで作業する必要があります。 indivは余分な文字と悪いチェックの問題を指摘した。あなたは、文字列の末尾にバック\0を入れていないので、あなたの代わりにmemcpyを使用する必要があるためstrcpyは失敗します：

void MyString::getline(istream& in) 
{ 
    char c; 
    erase(); 
    while(in.get(c) && c != '\n') 
    { 
     data[size++] = c; 
     if(size >= capacity) 
     { 
      capacity *= 2; 
      char *newData = new char[capacity]; 
      memcpy(newData, data, size); 
      delete[] data; 
      data = newData; 
     } 
    } 
    data[size] = '\0'; 
} 

---

int MyString::length() const 
{ 
    return strlen(data); 
} 

をsizeがすでに持っているので、これは単に時間を無駄にされました文字列の長さそれがインライン化できるように、実際にクラス定義で定義する必要があります。

int MyString::length() const 
{ 
    return size; 
} 

---

void MyString::grow() 
{ 
capacity=strlen(data)+1; 
MyString temp; 
temp=data; 
delete [] data; 
capacity*=2; 
data= new char[capacity]; 
} 

最初の行は、単に間違っています。 capacityは既に正しく設定されています。 tempは必要ありません。セグメンテーションフォールトがオンに起こるん何行

ostream& operator<<(ostream& out, const MyString& s) 
{ 
    return out << s.data; 
}