C++

Forum

 
Pair class exercise - help needed

DragonOsman (715)
Help needed in compiling code. Exercise for Pair class. 
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
 
#include <iostream>
#include <iomanip>
#include <vector>
#include <utility>
#include "../../cust_std_lib_facilities.h"

template<typename T1, typename T2>
class Pair
{
public:
	Pair(T1 first, T2 second)
		: first_elem{ first }, second_elem{ second } {}
	Pair()
		: first_elem{}, second_elem{} {}
	Pair(T1&&, T2&&);
	Pair(T1&, T2&);
	Pair &operator=(const Pair &other);
	Pair &operator=(Pair<T1, T2> &&other);
	Pair get(Pair &p) const;
	Pair make_pair(T1 x, T2 y);
	Pair &operator()(const T1 &x, const T2 &y);
	T1 get_first() const { return first_elem; }
	T2 get_second() const { return second_elem; }
private:
	T1 first_elem;
	T2 second_elem;
};

int main()
{
	std::vector<Pair<std::string, int>> my_table;
	std::string var1{ "var a" };
	std::string var2{ "var b" };
	std::string var3{ "var c" };
	std::string var4{ "var x" };
	std::string var5{ "var y" };
	std::vector<std::string> strs{ var1, var2, var3, var4, var5 };
	for (int i = 0; i < 5; ++i)
	{
		my_table.push_back(my_table[i].make_pair(strs[i], i * i + 1));
	}
	for (const auto &x : my_table)
	{
		std::cout << x.get_first() << std::setw(4) << std::right << x.get_second() << '\n';
	}
	keep_window_open();
}

template<typename T1, typename T2>
Pair<T1, T2>::Pair(T1 &&p1, T2 &&p2)
	: first_elem{std::move(p1)}, second_elem{std::move(p2)}
{	
}

template<typename T1, typename T2>
Pair<T1, T2>::Pair(T1 &p1, T2 &p2)
	: first_elem{ p1 }, second_elem{ p2 }
{
}

template<typename T1, typename T2>
Pair<T1, T2> &Pair<T1, T2>::operator=(const Pair &other)
{
	first_elem = other.first_elem;
	second_elem = other.second_elem;
	return *this;
}


template<typename T1, typename T2>
Pair<T1, T2> &Pair<T1, T2>::operator=(Pair<T1, T2> &&other)
{
	first_elem = std::forward<T1>(other.first_elem);
	second_elem = std::forward<T2>(other.second_elem);
	return *this;
}

template<typename T1, typename T2>
Pair<T1, T2> Pair<T1, T2>::get(Pair<T1, T2> &p) const
{
	return p.second_elem;
}

template<typename T1, typename T2>
Pair<T1, T2> Pair<T1, T2>::make_pair(T1 x, T2 y)
{
	return Pair<T1, T2>(std::forward<T1>(x), std::forward<T2>(y));
}

template<typename T1, typename T2>
Pair<T1, T2> &Pair<T1, T2>::operator()(const T1 &x, const T2 &y)
{
	return Pair<T1, T2>(std::forward<T1>(x), std::forward<T2>(y));
}


compiler errors:
1>chapter19ex3.cpp(98,9): error : call to constructor of 'Pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, int>' is ambiguous
1> return Pair<T1, T2>(std::forward<T1>(x), std::forward<T2>(y));
1> ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1>chapter19ex3.cpp(51,34) : note: in instantiation of member function 'Pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, int>::make_pair' requested here
1> my_table.push_back(my_table[i].make_pair(strs[i], i * i + 1));
1>C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.11.25503\include\xmemory0(856,4): error : call to implicitly-deleted copy constructor of 'Pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, int>'
1> _Objty(_STD forward<_Types>(_Args)...);
1> ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~

1>In file included from chapter19ex3.cpp:14:
1>In file included from C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.11.25503\include\vector:6:
1>C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.11.25503\include\xmemory(140,3): error : no matching function for call to 'construct'
1> allocator_traits<_Alloc>::construct(_Al, _Unfancy(_Dest), _STD move(*_First));
1> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1>C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.11.25503\include\xmemory(171,3) : note: in instantiation of function template specialization 'std::_Uninitialized_move_al_unchecked<Pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, int> *, Pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, int> *, std::allocator<Pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, int> > >' requested here
1> _Uninitialized_move_al_unchecked(_UFirst, _ULast, _UDest, _Al,
1> ^
1>C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.11.25503\include\vector(1924,3) : note: in instantiation of function template specialization 'std::_Uninitialized_move<Pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, int> *, Pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, int> *, std::allocator<Pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, int> > >' requested here
1> _Uninitialized_move(_First, _Last, _Dest, this->_Getal());
1> ^
1>C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.11.25503\include\vector(1934,3) : note: in instantiation of member function 'std::vector<Pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, int>, std::allocator<Pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, int> > >::_Umove_if_noexcept1' requested here
1> _Umove_if_noexcept1(_First, _Last, _Dest,
1> ^
1>C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.11.25503\include\vector(960,4) : note: in instantiation of member function 'std::vector<Pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, int>, std::allocator<Pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, int> > >::_Umove_if_noexcept' requested here
1> _Umove_if_noexcept(this->_Myfirst(), this->_Mylast(), _Newvec);
1> ^
1>chapter19ex3.cpp(51,12) : note: in instantiation of member function 'std::vector<Pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, int>, std::allocator<Pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, int> > >::push_back' requested here
1> my_table.push_back(my_table[i].make_pair(strs[i], i * i + 1));
1> ^
1>C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.11.25503\include\xmemory0(853,15) : note: candidate template ignored: substitution failure [with _Objty = Pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, int>, _Types = <Pair<std::basic_string<char, std::char_traits<char>, std::allocator<char> >, int>>]
1> static void construct(_Alloc&, _Objty * const _Ptr, _Types&&... _Args)
1> ^


What constructor am I missing? Please help with the other things that are wrong, too.
Last edited on
JayhawkZombie (738)
You do not have a copy constructor that takes a Pair object.

And try:
my_table.emplace_back(strs[i], i * i + 1); instead of my_table.push_back(my_table[i].make_pair(strs[i], i * i + 1));. Your code will crash if you leave it as it is.

Fixing that and changing that bit of code produces this output:
var a   1
var b   2
var c   5
var x  10
var y  17
Press any key to continue . . .


You can use default if you do not want to write every constructor and a default implementation will be sufficient.

You'll probably want a move constructor that takes a Pair rvalue reference, since you've already made similar constructors.
For make_pair and Pair(T1 first, T2 second) both parameters can be a const reference, like (const T1 &first, const T2 &second).
Thomas1965 (4571)
Could you show the text of this exercise. It look's a bit like a overkill to me.
As a rule of thumb you need copy and move constructors and assignment operators only when you use dynamic memory.

On line 40 my_table.push_back(my_table[i].make_pair(strs[i], i * i + 1)); you try to access an empty vector.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
 
#include <iostream>
#include <string>
#include <iomanip>
#include <vector>
#include <utility>

template<typename T1, typename T2>
class Pair
{
public:
  Pair(T1 first, T2 second)
    : first_elem{ first }, second_elem{ second } {}
  Pair()
    : first_elem{}, second_elem{} {}
  Pair(T1 &p1, T2 &p2);
  Pair get(Pair &p) const;
  Pair make_pair(T1 x, T2 y);
  Pair &operator()(const T1 &x, const T2 &y);
  T1 get_first() const { return first_elem; }
  T2 get_second() const { return second_elem; }
private:
  T1 first_elem;
  T2 second_elem;
};

int main()
{
  std::vector<Pair<std::string, int>> my_table;
  std::vector<std::string> vars = {"var a", "var b", "var c", "var x", "var y"};
  
  for (std::size_t i = 0; i < vars.size(); ++i)
  {
    my_table.push_back(Pair<std::string, int>(vars[i], i * i + 1));
  }
  for (const auto &x : my_table)
  {
    std::cout << x.get_first() << std::setw(4) << std::right << x.get_second() << '\n';
  }  
}

template<typename T1, typename T2>
Pair<T1, T2>::Pair(T1 &p1, T2 &p2)
  : first_elem{ p1 }, second_elem{ p2 }
{
}

template<typename T1, typename T2>
Pair<T1, T2> Pair<T1, T2>::get(Pair<T1, T2> &p) const
{
  return p.second_elem;
}

template<typename T1, typename T2>
Pair<T1, T2> Pair<T1, T2>::make_pair(T1 x, T2 y)
{
  return Pair<T1, T2>(std::forward<T1>(x), std::forward<T2>(y));
}

template<typename T1, typename T2>
Pair<T1, T2> &Pair<T1, T2>::operator()(const T1 &x, const T2 &y)
{
  return Pair<T1, T2>(std::forward<T1>(x), std::forward<T2>(y));
}

Output:

var a   1
var b   2
var c   5
var x  10
var y  17
DragonOsman (715)
Thanks, guys.

Is this good, then (also let me know if I should change my definition for make_pair())?
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
 
// Osman Zakir
// 10 / 23 / 2017
// Bjarne Stroustrup: Programming: Principles and Practice Using C++ 2nd Edition
// Chapter 19 Exercise 3
// Exercise Specifications:
/**
 * Write a template class Pair that can hold a pair of values of any type.
 * Use this to implement a simple symbol table like the one we used in the
 * calculator (ยง7.8).
 */

#include <iostream>
#include <iomanip>
#include <vector>
#include <utility>
#include "../../cust_std_lib_facilities.h"

template<typename T1, typename T2>
class Pair
{
public:
	Pair(T1 first, T2 second)
		: first_elem{ first }, second_elem{ second } {}
	Pair()
		: first_elem{}, second_elem{} {}
	Pair(T1&&, T2&&);
	Pair(Pair &&p) = default;
	Pair(T1&, T2&);
	Pair(const Pair &p) = default;
	Pair &operator=(const Pair &other);
	Pair &operator=(Pair<T1, T2> &&other);
	Pair get(Pair &p) const { return p.second_elem; }
	Pair make_pair(T1 x, T2 y);
	Pair &operator()(const T1 &x, const T2 &y);
	T1 get_first() const { return first_elem; }
	T2 get_second() const { return second_elem; }
private:
	T1 first_elem;
	T2 second_elem;
};

int main()
{
	std::vector<Pair<std::string, int>> my_table;
	std::vector<std::string> vars{ "var a", "var b", "var c", "var x", "var y" };
	for (std::size_t i = 0; i < vars.size(); ++i)
	{
		my_table.push_back(Pair<std::string, int>(vars[i], i * i + 1));
	}
	for (const auto &x : my_table)
	{
		std::cout << x.get_first() << std::setw(4) << std::right << x.get_second() << '\n';
	}
	keep_window_open();
}

template<typename T1, typename T2>
Pair<T1, T2>::Pair(T1 &&p1, T2 &&p2)
	: first_elem{std::move(p1)}, second_elem{std::move(p2)}
{	
}

template<typename T1, typename T2>
Pair<T1, T2>::Pair(T1 &p1, T2 &p2)
	: first_elem{ p1 }, second_elem{ p2 }
{
}

template<typename T1, typename T2>
Pair<T1, T2> &Pair<T1, T2>::operator=(const Pair &other)
{
	first_elem = other.first_elem;
	second_elem = other.second_elem;
	return *this;
}

template<typename T1, typename T2>
Pair<T1, T2> &Pair<T1, T2>::operator=(Pair<T1, T2> &&other)
{
	first_elem = std::forward<T1>(other.first_elem);
	second_elem = std::forward<T2>(other.second_elem);
	return *this;
}

template<typename T1, typename T2>
Pair<T1, T2> Pair<T1, T2>::make_pair(T1 x, T2 y)
{
	return Pair<T1, T2>(std::forward<T1>(x), std::forward<T2>(y));
}

template<typename T1, typename T2>
Pair<T1, T2> &Pair<T1, T2>::operator()(const T1 &x, const T2 &y)
{
	return Pair<T1, T2>(std::forward<T1>(x), std::forward<T2>(y));
}


Would there be an easy way to let the user enter the variables and initialize them for this? That's what the var_table in the calculator mentioned in the exercise specs is doing, after all.
JLBorges (13770)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
 
#include <iostream>
#include <string>

template < typename FIRST, typename SECOND > struct Pair
{
    template < typename T, typename U >
    Pair( T&& a, U&& b ) : first( std::forward<T>(a) ), second( std::forward<U>(b) ) {}

    template < typename T, typename U >
    Pair( const Pair<T,U>& that ) : first(that.first), second(that.second) {}

    template < typename T, typename U >
    Pair( Pair<T,U>&& that ) noexcept : first( std::move(that.first) ), second( std::move(that.second) ) {}

    template < typename T, typename U >
    Pair& operator= ( const Pair<T,U>& that ) { first = that.first ; second = that.second ; return *this ; }

    template < typename T, typename U >
    Pair& operator= ( Pair<T,U>&& that ) { first = std::move(that.first) ; second = std::move(that.second) ; return *this ; }

    ~Pair() = default ;

    // no class invariant can be identified; ergo public
    FIRST first ;
    SECOND second ;

    friend bool operator< ( const Pair& a, const Pair& b )
    { return a.first < b.first ? true : b.first < a.first ? false : a.second < b.second ; }

    friend bool operator> ( const Pair& a, const Pair& b ) { return b < a ; }

    // other overloaded relational operators
};

static_assert( __cplusplus >=  201703L, "C++17 is required" ) ;
// C++17: this deduction guide subsumes the functionality of the verbose make_pair
template < typename FIRST, typename SECOND > Pair( FIRST, SECOND ) -> Pair<FIRST,SECOND> ;

int main()
{
    const Pair p1 { 23, "hello" } ;
    const Pair< long long, std::string > p2 = p1 ;

    std::cout << std::boolalpha << ( Pair{ 123, 45.6 } < Pair{ 123, 45.62 } ) << '\n' ; // true
}

http://coliru.stacked-crooked.com/a/b6497c50735287e8
DragonOsman (715)
I don't have C++17 support in my compiler yet. So I can't do the stuff that requires it.
JLBorges (13770)
So replace it with the function make_Pair
DragonOsman (715)
Is my implementation of make_pair() good, or do I need to change it?
JLBorges (13770)
If make_pair<>() is to be a member of Pair<>, it should be a static member. Make it a non-member, placed in the same namespace as Pair<>, or it wont be of much practical use.

For perfect forwarding to work, forwarding references are required.

1
2
3
4
5
 
template<typename T1, typename T2>
Pair<T1,T2> make_pair( T1&& x, T2&& y ) 
{
	return Pair<T1,T2>( std::forward<T1>(x), std::forward<T2>(y) );
}
DragonOsman (715)
1
2
3
4
5
 
template<typename T1, typename T2>
Pair<T1, T2> make_pair(T1 &&x, T2 &&y)
{
	return Pair<T1, T2>(std::forward<T1>(x), std::forward<T2>(y));
}


Is that better?

And what way it would it be best to use it in?
Topic archived. No new replies allowed.