2014-12-05 51 views
2

我的應用程序中出現了一個問題,即我的PrintAll函數無法正常工作並最終導致應用程序崩潰。我的應用程序應該從文件讀取字符串並將它們插入到數組中。問題在於它讀取不正確,並最終導致我的應用程序崩潰。這裏就是我認爲問題在於:C++函數導致應用程序崩潰並且無法正常工作

int main() 
{ 
    LoadMovies(); 

    MovieList *movies = LoadMovies(); 
    //movies->MovieList::PrintAll(); 

    // // test methods for the Movie and MovieList classes 
     //PrintAllMoviesMadeInYear(movies, 1984); 
     //PrintAllMoviesWithStartLetter(movies, 'B'); 
     //PrintAllTopNMovies(movies, 5); 

    //delete movies; 
    return 0; 
} 

MovieList* LoadMovies() 
{ 
    vector<string> movies; 
    ReadMovieFile(movies); 
    MovieList ml = MovieList(movies.size()); 

    string name; 
    int year; 
    double rating; 
    int votes; 

    for (int i = 0; i < movies.size(); i++) 
    { 
     istringstream input_string(movies[i]); 
     getline(input_string, name, '\t'); 
     input_string >> year >> rating >> votes; 
     Movie movie (name, year, votes, rating); 
     ml.Add(movie); 
    } 
    ml.PrintAll(); 
} 

完整的例子:

/* 
* File: MovieStatsProgram.cpp 
* Author: 
* Date: 
* =============================================================== 
* This is a console app to test the Movie and MovieList classes. 
* 
* TODO: 
* 
* You need to finish the implementation of the loadMovies method 
* to create and initialize the MovieList object. 
* 
* You also need to create three static methods: 
* 
* PrintAllMoviesMadeInYear - it will print all the movies made in a 
* given year once sort in alphabetical order and once sorted by the number 
* of votes with the movie with the most number of votes printed first. 
* 
* PrintAllMoviesWithStartLetter - it will print all the movies started with 
* a given letter sorted in alphabetical order 
* 
* PrintAllTopNMovies - it will display the top N movies based on the number of 
* votes 
*/ 

#include <iostream> 
#include <sstream> 
#include <vector> 
#include <string> 
#include <iomanip> 
#include <fstream> 

using namespace std; 

class Movie { 
public: 
    Movie(); 
    Movie(string n, int y, int v, double r); 
    string get_name(); 
    void set_name(string n); 
    int get_year(); 
    void set_year(int y); 
    int get_votes(); 
    void set_votes(int v); 
    double get_rating(); 
    void set_rating(double r); 
    string PrintMovie(); 

private: 
    string name; 
    int year_made; 
    int votes; 
    double rating; 

}; 

Movie::Movie() { 
    name = "null"; 
    year_made = 0; 
    votes = 0; 
    rating = 0.0; 
} 

Movie::Movie(string n, int y, int v, double r) { 
    name = n; 
    year_made = y; 
    votes = v; 
    rating = r; 
} 

string Movie::get_name() { 
    return name; 
} 

void Movie::set_name(string n) { 
    name = n; 
} 

int Movie::get_year() { 
    return year_made; 
} 

void Movie::set_year(int y) { 
    year_made = y; 
} 

int Movie::get_votes() { 
    return votes; 
} 

void Movie::set_votes(int v) { 
    votes = v; 
} 

double Movie::get_rating() { 
    return rating; 
} 

void Movie::set_rating(double r) { 
    rating = r; 
} 

string Movie::PrintMovie() { 
    cout << fixed << setprecision(1) << rating << "\t\t" << votes << "\t\t" << "(" << 
      year_made << ")" << "\t" << name << endl; 
} 

class MovieList { 
public: 
    MovieList(int size); 
    ~MovieList(); 
    int Length(); 
    bool IsFull(); 
    void Add(Movie const& m); 
    string PrintAll(); 

private: 
    Movie* movies; 
    int last_movie_index; 
    int movies_size; 
    int movie_count = 0; 

}; 

MovieList::MovieList(int size) { 
    movies_size = size; 
    movies = new Movie[movies_size]; 
    last_movie_index = -1; 
} 

MovieList::~MovieList() { 
    delete [] movies; 
} 

int MovieList::Length() { 
    return last_movie_index; 
} 

bool MovieList::IsFull() { 
    return last_movie_index == movies_size; 
} 

void MovieList::Add(Movie const& m) 
{ 
    if (IsFull()) { 
     cout << "Cannot add movie, list is full" << endl; 
     return; 
    } 

    ++last_movie_index; 
    movies[last_movie_index] = m; 
} 

string MovieList::PrintAll() { 
    for (int i = 0; i < last_movie_index; i++) { 
     movies[last_movie_index].Movie::PrintMovie(); 
     //cout << movies[last_movie_index] << endl; 
    } 
} 

void ReadMovieFile(vector<string> &movies); 
MovieList* LoadMovies(); 

enum MovieSortOrder 
{ 
    BY_YEAR = 0, 
    BY_NAME = 1, 
    BY_VOTES = 2 
}; 

int main() 
{ 
    LoadMovies(); 

    MovieList *movies = LoadMovies(); 
    //movies->MovieList::PrintAll(); 

    // // test methods for the Movie and MovieList classes 
     //PrintAllMoviesMadeInYear(movies, 1984); 
     //PrintAllMoviesWithStartLetter(movies, 'B'); 
     //PrintAllTopNMovies(movies, 5); 

    //delete movies; 
    return 0; 
} 

MovieList* LoadMovies() 
{ 
    vector<string> movies; 
    ReadMovieFile(movies); 
    MovieList ml = MovieList(movies.size()); 

    string name; 
    int year; 
    double rating; 
    int votes; 

    for (int i = 0; i < movies.size(); i++) 
    { 
     istringstream input_string(movies[i]); 
     getline(input_string, name, '\t'); 
     input_string >> year >> rating >> votes; 
     Movie movie (name, year, votes, rating); 
     ml.Add(movie); 
    } 
    ml.PrintAll(); 
} 

void ReadMovieFile(vector<string> &movies) 
{ 
    ifstream instream; 
    instream.open("imdbtop250.txt"); 
    if (instream.fail()) 
    { 
     cout << "Error opening imdbtop250.txt" << endl; 
     exit(1); 
    } 


    while (!instream.eof()) 
    { 
     string movie; 
     getline(instream, movie); 
     movies.push_back(movie); 
    } 

    instream.close(); 
} 

當我在主函數中使用MovieList :: PrintAll,我的函數只是崩潰,當我把它放在LoadMovies功能,它會在崩潰之前錯誤地讀取和添加數據。列表的大小爲251,應用程序將只讀取相同的數據251次。

+2

該函數,LoadMovies()不返回任何東西。你沒有得到編譯器警告?注意警告並解決它們。 – 2014-12-05 21:58:06

+1

請提供[最小,完整和可驗證示例](https://stackoverflow.com/help/mcve)。 – 5gon12eder 2014-12-05 21:58:48

+0

@BradS。我看到我在LoadMovies上錯過了一個返回,但是在這種情況下我應該返回什麼? – andayn 2014-12-05 22:07:09

回答

1

你有兩個部分問題:

1:布拉德的陳述的,你的函數沒有返回。這是一個禁忌。

MovieList* LoadMovies() 
{ 
    MovieList ml = MovieList(movies.size()); 
    // Your function returns a pointer to a MovieList, so... 
    return &ml; 
} 

因此,問題2是你要返回一個指向你在函數堆棧中創建的東西的指針。當你嘗試在你的函數之外訪問它時,你會遇到未定義的行爲。

選項1:

MovieList* ml = new MovieList(movies.size()); 
return ml; 

現在,您需要刪除毫升時,即可大功告成瓦特/它。

選項2: 改變你的函數返回一個非指針...然後你沒有管理內存的麻煩。

編輯:試試這個

int main() 
{ 
    // Don't need this 
    // LoadMovies(); 

    MovieList *movies = LoadMovies(); 

    // Uncommented this 
    delete movies; 
return 0; 
} 

MovieList* LoadMovies() 
{ 
    vector<string> movies; 
    ReadMovieFile(movies); 
    // CHANGE 
    MovieList* ml = new MovieList(movies.size()); 
    // CHANGE 

    string name; 
    int year; 
    double rating; 
    int votes; 

    for (int i = 0; i < movies.size(); i++)  
    { 
     istringstream input_string(movies[i]); 
     getline(input_string, name, '\t');  
     input_string >> year >> rating >> votes; 
     Movie movie (name, year, votes, rating); 
     ml.Add(movie); 
    } 
    ml.PrintAll(); 
    // CHANGE 
    return ml; 
} 
+0

我認爲問題在於我的PrintAll函數,因爲現在我的應用程序將只打印它從文件中讀取的最後一個項目並崩潰。 LoadMovies應該正確添加元素到數組中嗎?我甚至無法將LoadMovies切換到cout << movies [last_movie_index] << endl; – andayn 2014-12-05 22:18:41

+0

您正在執行PrintAll(),然後從函數中返回(無)。這可能是它崩潰的地方。 – 2014-12-06 07:42:37

0

MovieList類有一個根本性的問題。這涉及到光在這條線:

MovieList ml = MovieList(movies.size());

你MovieList類有一個成員是動態分配的內存的指針。一旦你有了這個,你必須通過創建一個用戶定義的拷貝構造函數和賦值操作符來管理拷貝和賦值。

最簡單的解決方法是使用std::vector<Movie>而不是Movie *作爲MovieList的成員變量。然後複製分配是免費的,您無需執行其他功能。

但是,如果你不能因爲某些原因使用std::vector,下列功能可以添加:

class MovieList { 
public: 
    //... 
    MovieList(const MovieList& m); 
    MovieList& operator=(MovieList m); 
    //... 
}; 

#include <algorithm> 
//...  
// copy constructor 
MovieList::MovieList(const MoveList& m) { 
    movies_size = m.size; 
    movie_count = m.movie.count; 
    last_movie_index = m.last_movie_index; 
    movies = new Movie[movies_size]; 
    for (int i = 0; i < movies_size; ++i) 
     movies[i] = m.movies[i]; 
} 
//... 
// assignment operator 
MovieList& MovieList::operator=(MoveList m) { 
    std::swap(m.movie_size, movie_size); 
    std::swap(m.last_movie_index, last_movie_index); 
    std::swap(m.movies, movies); 
    std::swap(m.movie_count, moviE_count); 
    return *this; 
} 

來形容這個給你最簡單的方法是不來形容這些事。對你來說最好的辦法就是使用你的調試器,並在這些函數中添加一個斷點,並逐步完成代碼。當你點擊上面提到的那一行時,你會看到複製構造函數被調用 - 那麼你可以看到它正在做什麼。

賦值運算符是當您將現有的MovieList分配給另一個MovieList時調用的函數。它通過copy/swap成語實現。這依賴於工作副本構造函數(上面提供)和析構函數(您已在代碼中提供)。它的工作原理是創建一個臨時MovieList,並使用臨時MovieList交換當前MovieList的內部。關於如何工作,SO上有很多線程。

至於爲什麼你需要以上這些功能的原因是,沒有上述功能,行:

MovieList ml = MovieList(movies.size());

將創建兩個MovieList對象,一個臨時的和一個非暫時性的,但是movies兩者的指針都將指向相同的內存。當臨時被銷燬時,析構函數被調用,從而刪除movies指向的內存。現在你有m1指向已經冒煙的內存。當您嘗試使用m1時發生壞消息。

上面的用戶定義的複製和分配函數正確地複製對象,以便爲movies獲得兩個不同的內存分配,以便在調用析構函數時刪除的內存對於該對象是唯一的。

同樣,如果您使用std::vector並且不得不編寫副本/賦值運算符,所有這些都會得到緩解。