2014-12-05 88 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并且不得不编写副本/赋值运算符,所有这些都会得到缓解。

相关问题