Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue with Exercise 13.21 #304

Closed
opensourceDA opened this issue Jul 22, 2015 · 15 comments
Closed

Issue with Exercise 13.21 #304

opensourceDA opened this issue Jul 22, 2015 · 15 comments
Assignees

Comments

@opensourceDA
Copy link

In the answer to Exercise 13.21 it is suggested that the copy constructor of QueryResult class should be declared as deleted. But in the implementation of TextQuery::query function(Exercise 12.32) we use the copy constructor to return a QueryResult type. This results in error. So, the copy constructor for QueryResult class should be deleted.

@Mooophy
Copy link
Owner

Mooophy commented Jul 22, 2015

Sorry I did't get you.

@opensourceDA
Copy link
Author

In exercise 13.21 the copy constructor of QueryResult should not be delete. We need the copy constructor in query function of TextQuery class.

@opensourceDA
Copy link
Author

Exercise 13.21:link!

TextQuery(const TextQuery&) = delete;
TextQuery& operator=(const TextQuery&) = delete;

QueryResult(const QueryResult&) = delete;    //wrong
QueryResult& operator=(const QueryResult&) = delete;  //wrong

Reason:

Need copy constructor here : Exercise 12.32link!

QueryResult TextQuery::query(const string& str) const 
{
    // use static just allocate once.
    static shared_ptr<std::set<StrBlob::size_type>> nodate(new std::set<StrBlob::size_type>);
    auto found = result.find(str);
    if (found == result.end()) return QueryResult(str, nodate, input);
    else return QueryResult(str, found->second, input);      //   <--  need copy constructor here
}

@pezy
Copy link
Collaborator

pezy commented Jul 23, 2015

else return QueryResult(str, found->second, input);      //   <--  need copy constructor here

why ❓

@opensourceDA
Copy link
Author

A return is initialized by copy constructor right? Anyway i tried compiling and i got a error so i reported.
I have attached screenshots of the error. Have a look.
screenshot from 2015-07-23 17 07 55
screenshot from 2015-07-23 17 07 01

@Mooophy
Copy link
Owner

Mooophy commented Jul 23, 2015

@opensourceDA
Which compiler are you using?

@Mooophy
Copy link
Owner

Mooophy commented Jul 23, 2015

Paste detailed compiler info please.

@opensourceDA
Copy link
Author

Using g++ (Ubuntu 4.8.4-2ubuntu1~14.04) 4.8.4 !

@Mooophy
Copy link
Owner

Mooophy commented Jul 23, 2015

OK. I'll look into it now.

@opensourceDA
Copy link
Author

Let me know the cause for error whenever u find out please! Thnx

@Mooophy
Copy link
Owner

Mooophy commented Jul 23, 2015

Please paste the two files you tried to compile, including both the .h and the .cpp file.

@opensourceDA
Copy link
Author

http://pastebin.com/g6p4uNV5 -- this is the class

http://pastebin.com/vNCjGAVf -- driver

@Mooophy
Copy link
Owner

Mooophy commented Jul 23, 2015

Ok.I think I know what's going on. Basically, we need add one more line into it:

QueryResult(QueryResult &&) noexcept = default;

For such case, copy constructor works, just as @opensourceDA expected.
The best match, however, should be move constructor, which has better performance. But the current code has disabled compiler to generate synthesized move constructor for us. So if we want move semantics as expected when copy constructor deleteed explicitly, we must explicitly mark the default move constructor with = default.

I tested the code below with Vs 2013, Vs 2015 RC and G++ 4.82. All compiled.

#ifndef MY_CLASS
#define MY_CLASS

#include<iostream>
#include<sstream>
#include<fstream>
#include<map>
#include<set>
#include<vector>
#include<string>
#include<memory>

class QueryResult;
std::ostream& print(std::ostream&, const QueryResult&);

class TextQuery {
public:
    //typedef
    using line_no = std::vector<std::string>::size_type;
    //constructor
    TextQuery(std::ifstream&);
    //Query function
    QueryResult query(const std::string&) const;
    TextQuery(const TextQuery&) = delete;
    TextQuery& operator=(const TextQuery&) = delete;
private:
    //data members
    std::shared_ptr<std::vector<std::string>> in_file;
    std::map<std::string, std::shared_ptr<std::set<line_no>>> word_occr;
};

//TextQuery constructor
TextQuery::TextQuery(std::ifstream& input) : in_file(new std::vector<std::string>) {
    for (std::string text; getline(input, text); in_file->push_back(text)) {
        int cur_line_no = in_file->size() + 1;
        std::string word;
        for (std::istringstream line(text); line >> word; ) {
            auto &line_nums = word_occr[word];
            if (!line_nums)
                line_nums.reset(new std::set<TextQuery::line_no>);
            line_nums->insert(cur_line_no);
        }
    }
}

class QueryResult {
    friend std::ostream& print(std::ostream&, const QueryResult&);
public:
    //constructor
    QueryResult(std::string s,
        std::shared_ptr<std::vector<std::string>> f,
        std::shared_ptr<std::set<TextQuery::line_no>> l)
        : sought(s), file(f), lines(l) { }
    QueryResult(const QueryResult&) = delete;
    QueryResult& operator=(const QueryResult&) = delete;



    QueryResult(QueryResult &&) noexcept = default; //!! here!!



    //member functions
    std::set<TextQuery::line_no>::iterator begin() { return lines->begin(); }
    std::set<TextQuery::line_no>::iterator end() { return lines->end(); }
    std::shared_ptr<std::vector<std::string>> get_file() { return file; }
private:
    //data members
    std::string sought;             //word sought by this query
    std::shared_ptr<std::vector<std::string>> file;         //file we are searching in
    std::shared_ptr<std::set<TextQuery::line_no>> lines;    //lines sought is found on
};

QueryResult TextQuery::query(const std::string& s) const {
    static std::shared_ptr<std::set<line_no>> nodata(new std::set<line_no>);
    auto found = word_occr.find(s);
    if (found == word_occr.end())
        return QueryResult(s, in_file, nodata);
    else
        return QueryResult(s, in_file, found->second);
}

std::ostream& print(std::ostream& out, const QueryResult& qr) {
    out << qr.sought << " : " << qr.lines->size() << std::endl;
    return out;
}

#endif

Please try it.

@opensourceDA
Copy link
Author

Yeah, works. Then to the answer of Exercise 13.21, please add this detail. thanks

@Mooophy Mooophy self-assigned this Jul 23, 2015
@Mooophy
Copy link
Owner

Mooophy commented Jul 23, 2015

Thx for reporting.

Mooophy added a commit that referenced this issue Jul 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants