2016-11-30 19 views
0

私は学校の割り当てのための簡単な飛行予約システムを書こうとしています。私は、サイズを決定することなく、動的に配列を作成する必要があります。配列のサイズを調べなければならないので、クラス内でcountという名前の整数変数を宣言しました。コピーコンストラクタと2つのゲッターを持つフライトクラスもあります。それから私は、私はまた、特定のフライトを示しshowFlight(const int flightCode)方法を持って、次の方法配列の動的割り当て

void ReservationSystem::addFlight(const int flightNo, const int rowNo, const int seatNo) { 
    if (count == 0) { 
     Flight *tmp = new Flight(flightNo, rowNo, seatNo); 
     listOfFlights = new Flight*[count+1]; 
     listOfFlights[count] = tmp; 
     count++; 
    } else { 
     bool check = true; 
     for (int i = 0; i < count && check; i++) { 
      if (listOfFlights[i]->getFlightNo() == flightNo) { 
       std::cout << "There is already a flight with that flight code" << std::endl; 
       check = false; 
      } 
     } 

     if (check) { 
      Flight *tmp = new Flight(flightNo, rowNo, seatNo); 
      Flight** tmparr = new Flight*[count + 1]; 

      for (int i = 0; i < count; i++) { 
       Flight *f = new Flight(*listOfFlights[i]); 
       tmparr[i] = f; 
      } 

      tmparr[count + 1] = tmp; 

      for (int i = 0; i < count; i++) { 
       delete listOfFlights[i]; 
      } 

      delete listOfFlights; 
      listOfFlights = tmparr; 
      count++; 
     } 


    } 

} 

を書いた:

void ReservationSystem::showFlight(const int flightNo) { 
    bool check = true; 
    for (int i = 0; i < count; i++) { 
     if (listOfFlights[i]->getFlightNo() == flightNo) { 
      std::cout << "Flight " << listOfFlights[i]->getFlightNo() << " has " << listOfFlights[i]->getAvailableSeats() << " available seats" << std::endl; 
      listOfFlights[i]->printSeats(); 
      check = false; 
     } 
    } 
} 

これは私のデフォルトコンストラクタで、Flightクラスのコンストラクタをコピーします。

Flight::Flight(const int flightNo, const int rowNo, const int seatNo) { 
    flight = flightNo; 
    row = rowNo; 
    seat = seatNo; 
    available = rowNo * seatNo; 
    flightPlan = new char*[seatNo]; 

    // initialize the flight plan to all seats available 
    for(int i = 0; i < seatNo; ++i) flightPlan[i] = new char[rowNo]; 

    for(int i = 0; i < seatNo; ++i) { 
     for(int j = 0; j < rowNo; ++j) flightPlan[i][j] = 'o'; 
    } 
} 

Flight::Flight(const Flight &obj) { 
    const int flight = obj.flight; 
    const int row = obj.row; 
    const int available = obj.available; 
    char** flightPlan = obj.flightPlan; 

} 

しかし、if (listOfFlights[i]->getFlightNo() == flightNo)のxcodeはEXC_BAD_ACCESSエラーを返します。私はこれの背後にある理由は、配列の何かがヌルに向いているオブジェクトがないので、私のaddFlight()メソッドの誤動作だと思いますか?また、getFlightNo()メソッドに到達できないため、このエラーがスローされます。

これはC++で初めてのことですので、私は完全にn00bであり、ひどく間違えることがあります。どんな助けでも大歓迎です。

+2

コピーコンストラクタは実際には間違っているように見えますが、未使用のローカル変数を設定するだけです。 –

+0

'std ::'コンテナクラスを使わないのはなぜですか? 'std :: vector <>'のように? – nvoigt

+1

あなたの学校があなたに配列とポインタ、そして配列と 'new []'を指すポインタの配列、そして 'std :: vector'の前のすべてのものをあなたに教えている場合、彼らはあなたをリッピングしています。 –

答えて

0

flightPlanについては、Flight

私はそれはデストラクタで削除したのか?それは、コンストラクタ

flightPlan = new char*[seatNo]; 

に割り当てられ、それがコピーコンストラクタ

char** flightPlan = obj.flightPlan; 

にコピーされる参照

もしそうなら、あなたはそれを対処新しい配列に古い配列からFlightを翻訳(そこにそれを行うためのより良い方法がたくさんありますが、私は今あなたのエラーを探しています)

 Flight** tmparr = new Flight*[count + 1]; 

     for (int i = 0; i < count; i++) { 
      Flight *f = new Flight(*listOfFlights[i]); 
      tmparr[i] = f; 
     } 

     // ... 

     for (int i = 0; i < count; i++) { 
      delete listOfFlights[i]; 
     } 

(コピー済み)FlightflightPlanへのポインタが削除された領域を指しています。 EXC_BAD_ACCESSを使用するとかなり安心です。

p.s .:追加の提案:可能であれば、割り当てられたメモリを直接使用しないようにしてください。 listOfFlightsstd::vector<Fligth>を使用することを検討してください。

0

問題1:

  Flight *tmp = new Flight(flightNo, rowNo, seatNo); 
      Flight** tmparr = new Flight*[count + 1]; 

      for (int i = 0; i < count; i++) { 
      // **** 
      // **** You are trying to copy a flight. Why? You have an array of 
      // **** Flight pointers. You need to make a room for one more poniter. 
      // **** You can continue pointing at the same flights as before. 
      // **** You don't need an array of brand new flights. 
      // **** 
       Flight *f = new Flight(*listOfFlights[i]); // <---- WRONG 
       Flight *f = listOfFlights[i]; // <------------------ BETTER 
       tmparr[i] = f; 
      } 

     // **** This is not needed any more 
     // **** 
     for (int i = 0; i < count; i++) { 
      delete listOfFlights[i]; 
     } 

問題2:

  // **** 
      // **** Learn to count. There are count+1 elements in your new array. 
      // **** The first index is 0. The last index is count, not count+1. 
      // **** You did get it right in the count==0 case. 
      // **** 
      tmparr[count + 1] = tmp; //<--- WRONG 
      tmparr[count] = tmp; // <------ RIGHT 

問題3

// **** 
// **** This copies nothing. It just creates and then forgets a bunch of local variables. 
// **** You probably don't need a copy constructor at all because 
// **** you don't need to copy flight any more. 

Flight::Flight(const Flight &obj) { 
    const int flight = obj.flight; 
    const int row = obj.row; 
    const int available = obj.available; 
    char** flightPlan = obj.flightPlan;  
} 

// **** Declare the copy constructor deleted instead. 

Flight::Flight(const Flight &) = delete; 

// **** If they are making you to use an old compiler that doesn't understand delete here 
// **** Then do this instead: 

private: Flight(const Flight &); // no implementation 

// **** You may want to do the same with the assignment operator 

// **** If you think do need a copy constructor for some reason, think again. 

// **** OK, if you still think you need a copy constructor, 
// **** make sure you DO NOT do this: 

flightPlan = obj.flightPlan 

// **** or an equivalent. You need a brand new copy of all of your arrays. 
// **** Also maje sure to define a copy assignment operator. 
// **** regardless, you need a destructor. Make sure you delete the arrays there. 

注、ここでは、ここで一つ一つのエラーが簡単にstd::vectorを使用して代わりにすることで回避されている可能性が手動で管理されるアレイ。

0

私はこのような愚かな質問をすることをお詫び申し上げます。最終的に問題は解決されます。明らかに、あなたがコピーしたばかりのポインタを削除することは悪く、コードに問題を引き起こす可能性があります。

ありがとうございます。

関連する問題