C++

Forum

 
smart menu

Pages: 123
globaltourist (201)
Hello ! I have made a smart menu that does the following:
1) allows to add a function with a name tied to it.
2) allows deleting an option by name.
3) set menu position
4) resizes the menu size according to the longest option name.

The question is did i code it effectively or are there places i can optimize ?

the code:

menu.h:
-------
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
 
/*
Copyright <2017> <Daniel Rossinsky>

Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"),
to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense,
and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
*/

#ifndef MENU_H
#define MENU_H

#include"Insertion_map.h"
#include"Console.h"

using funcPtr = void(*)(); 

class Menu
{
private:
	Insertion_map<std::string, funcPtr>	m_options;
	int							m_longestOpt{};
	int							m_menuHight{};
	int							m_menuLength;
	int							m_menuXPos;
	int							m_menuYPos;

	void printMenu(int curXPos, int curYPos);
	void executeOption(intPair coords);

public:
	explicit Menu(int x = 0, int y = 0)
	{
		setMenuXY(x, y);
	}

	void deleteOption(const std::string& opt) { if(m_options.erase(opt)) m_menuHight -= 5; }
	void addOption(const std::string& opt, const funcPtr func);
	void setMenuXY(int x, int y);
	void start();
};

#endif



menu.cpp:
---------
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
 
#include"Menu.h"
#include<string>

static unsigned const char selectionArrow{ static_cast<unsigned char>(175) };
static const int		   yJumpRange    { 5 };
static const int		   lengthPadding { 4 };
static const int		   optionHight   { 3 };
static const int		   xJumpRange    { 0 };

/*__MENU__*/
/*private_function_definitions_start*/ 
void Menu::printMenu(int curXPos, int curYPos) 
{
	Console::drawBox(m_menuHight, m_menuLength, curXPos, curYPos);
	curXPos += 2;
	curYPos += 1;
	for (int index{}; index < static_cast<int>(m_options.size()); ++index)
	{
		Console::drawBox(optionHight, m_longestOpt, curXPos, curYPos);
		Console::gotoxy(curXPos + 2, curYPos + 2);
		std::cout << m_options[index].first;
		curYPos += yJumpRange;
	}//end of for
}

void Menu::executeOption(intPair coords) 
{
	int index{};
	while (coords.second != (m_menuYPos + 3))
	{
		++index;
		coords.second -= yJumpRange;
	}//end of while
	m_options[index].second();
}
/*private_function_definitions_end*/

/*public_function_definitions_start*/
void Menu::addOption(const std::string& opt, const funcPtr func)
{
	if (m_options.emplace_back(opt, func))
	{
		m_menuHight += 5;
		if ((static_cast<int>(opt.length()) + lengthPadding) > m_menuLength)
		{
			m_menuLength = static_cast<int>(opt.length()) + lengthPadding;
			m_longestOpt = static_cast<int>(opt.length());
		}//end of if
	}//end of if
}

void Menu::setMenuXY(int x, int y)
{
	if (x < 0)
	{
		m_menuXPos = -x;
		std::cout << "x value was made positive.\n";
	}//end of if
	else m_menuXPos = x;
	if (y < 0)
	{
		m_menuYPos = -y;
		std::cout << "y value was made positive.\n";
	}//end of if
	else m_menuYPos = y;
}

void Menu::start()
{
	if (static_cast<int>(m_options.size()) == 0) 
		std::cout << "no options found, unable to print menu.\n";
	else
	{
		printMenu(m_menuXPos, m_menuYPos);
		executeOption(Console::navigation(intPair(xJumpRange, yJumpRange),
				                          intPair(m_menuXPos + 1, m_menuXPos + 1),
										  intPair(m_menuYPos + 3, (m_menuYPos + 3) + yJumpRange * (static_cast<int>(m_options.size()) - 1)),
										  selectionArrow));
		/*
		(m_menuYPos + 3): is the y coordinate of the left thin line between the menu border and the options.
		m_options.size()) - 1: is used because the selectionArrow will already point to the first option.
		multiplying yJumpRange by the previously mentioned options.size()) - 1 is representing the range
		between the center of the first and last option and we add (m_menuYPos + 3) to it so that the 
		selectionArrow will allways point to the middle of the options.
		*/
	}//end of else
}
/*public_function_definitions_end*/
/*__MENU__*/

Last edited on
globaltourist (201)
Constants.h:
---------------
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
 
#ifndef CONSTANTS_H
#define CONSTANTS_H

namespace Shapes
{
	extern unsigned const char BRAP;
	extern unsigned const char TRAP;
	extern unsigned const char BLAP;
	extern unsigned const char TLAP;
	extern unsigned const char HP;
	extern unsigned const char VP;
}

/*
#########
meanings:
#########
BRAP - Bottom right angle piece
TRAP - Top right angle piece
BLAP - Bottom left angle piece
TLAP - Top left angle piece
HP   - Horizontal piece
VP   - Vertial piece
SA   - Selection arrow
*/

#endif


Constants.cpp:
-----------------
1
2
3
4
5
6
7
8
9
10
11
 
#include"Constants.h"

namespace Shapes
{
	extern unsigned const char BRAP{ static_cast<unsigned char>(188) };
	extern unsigned const char TRAP{ static_cast<unsigned char>(187) };
	extern unsigned const char BLAP{ static_cast<unsigned char>(200) };
	extern unsigned const char TLAP{ static_cast<unsigned char>(201) };
	extern unsigned const char HP  { static_cast<unsigned char>(205) };
	extern unsigned const char VP  { static_cast<unsigned char>(186) };
}
Last edited on
globaltourist (201)
Console.h:
------------
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
 
#ifndef CONSOLE_GRAPHICS_H
#define CONSOLE_GRAPHICS_H
#include<utility>

using intPair = std::pair<int, int>;

namespace Console 
{
	intPair navigation(const intPair& jumpLength,
		               const intPair& xRange,
		               const intPair& yRange,
					   unsigned const char symbol = ' ');
	void putSymbol(int xPos, int yPos, const char symbol);
	void drawBox(int hight, int length, int x, int y);
	void gotoxy(int x, int y);
	void clrScr();
}

#endif


Console.cpp:
--------------
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
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
 
#include"Console.h" 
#include"Constants.h"
#include<iostream>
#include<Windows.h>
#include<conio.h>

static const int downKey { 80 };
static const int rightKey{ 77 };
static const int leftKey { 75 };
static const int upKey   { 72 };
static const int enterKey{ 13 };
static const int padding { 2 };

intPair Console::navigation(const intPair& jumpLength,
	                        const intPair& xRange,
	                        const intPair& yRange,
							unsigned const char symbol)
{
	int curX{ xRange.first };
	int curY{ yRange.first };
	int buffer;
	do {
		putSymbol(curX, curY, symbol);
		gotoxy(curX, curY);
		buffer = static_cast<int>(_getch());
		if (buffer == enterKey) return intPair(curX, curY);
		switch (static_cast<int>(_getch()))
		{
		case upKey: 
		{
			if ((curY - jumpLength.second) < yRange.first)
			{
				curY = yRange.second;
				putSymbol(curX, yRange.first, ' ');
			}//end of if
			else
			{
				curY -= jumpLength.second;
				putSymbol(curX, curY + jumpLength.second, ' ');
			}//end of else
		}//end of case 
		break;
		case downKey: 
		{
			if ((curY + jumpLength.second) > yRange.second)
			{
				curY = yRange.first;
				putSymbol(curX, yRange.second, ' ');
			}//end of if
			else
			{
				curY += jumpLength.second;
				putSymbol(curX, curY - jumpLength.second, ' ');
			}//end of else
		}//end of case 
		break;
		case leftKey: 
		{
			if ((curX - jumpLength.first) < xRange.first)
			{
				curX = xRange.second;
				putSymbol(xRange.first, curY, ' ');
			}//end of if
			else
			{
				curX -= jumpLength.first;
				putSymbol(curX + jumpLength.first, curY, ' ');
			}//end of else
		}//end of case 
		break;
		case rightKey: 
		{
			if ((curX + jumpLength.first) > xRange.second)
			{
				curX = xRange.first;
				putSymbol(xRange.second, curY, ' ');
			}//end of if
			else
			{
				curX += jumpLength.first;
				putSymbol(curX - jumpLength.first, curY, ' ');
			}//end of else
		}//end of case 
		break;
		}//end of switch
	} while (true);
}

inline void Console::putSymbol(int xPos, int yPos, const char symbol)
{
	gotoxy(xPos, yPos);
	std::cout << symbol;
}

void Console::drawBox(int hight, int length, int x, int y)
{
	gotoxy(x, y);
	std::cout << Shapes::TLAP;
	for (int i{}; i < (length + padding); ++i) std::cout << Shapes::HP; 
	std::cout << Shapes::TRAP;

	for (int i{}; i < hight; ++i)
	{
		gotoxy(x, ++y);
		std::cout << Shapes::VP;
		gotoxy(x + length + 3, y); // x + length + 3 represents the furthest boarder						   
		std::cout << Shapes::VP;
	}//end of for

	gotoxy(x, ++y);
	std::cout << Shapes::BLAP;
	for (int i{}; i < (length + padding); ++i) std::cout << Shapes::HP; 
	std::cout << Shapes::BRAP;
}

inline void Console::gotoxy(int x, int y)
{
	COORD pos = { static_cast<short>(x), static_cast<short>(y) };
	SetConsoleCursorPosition(GetStdHandle(STD_OUTPUT_HANDLE), pos);
}

void Console::clrScr()
{
	CONSOLE_SCREEN_BUFFER_INFO csbi;
	HANDLE                     hStdOut;
	DWORD                      count;
	DWORD                      cellCount;
	COORD                      homeCoords = { 0, 0 };

	hStdOut = GetStdHandle(STD_OUTPUT_HANDLE);						
	if (hStdOut == INVALID_HANDLE_VALUE) return;

	if (!GetConsoleScreenBufferInfo(hStdOut, &csbi)) return;
	cellCount = csbi.dwSize.X *csbi.dwSize.Y;

	if (!FillConsoleOutputCharacter(
		hStdOut,
		(TCHAR) ' ',
		cellCount,
		homeCoords,
		&count
	)) return;

	SetConsoleCursorPosition(hStdOut, homeCoords);
}



Last edited on
globaltourist (201)
source.cpp:
-------------

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
 
#include"Menu.h"

void printA()
{
	Console::clrScr();
	std::cout << "A\n";
}

void printB()
{
	Console::clrScr();
	std::cout << "B\n";
}

void printC()
{
	Console::clrScr();
	std::cout << "C\n";
}

void printD()
{
	Console::clrScr();
	std::cout << "D\n";
}

int main() {
	
	Menu m;
	m.setMenuXY(10, 2);
	funcPtr fPtr = printA;
	m.addOption("print A", fPtr);
	fPtr = printB;
	m.addOption("option print B", fPtr);
	fPtr = printC;
	m.addOption("print C", fPtr);
	fPtr = printD;
	m.addOption("print D", fPtr);
	m.deleteOption("print D");
	m.start();
	
	return 0;
}
globaltourist (201)
Insertion_map.h:
--------------------
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
 
#ifndef INSERTION_MAP
#define INSERTION_MAP

#include<map>
#include<vector>
#include<iostream>

template<typename varOne, typename varTwo>
class Insertion_map
{
private:
	std::vector<varOne>		 m_insertionOrder;
	std::map<varOne, varTwo> m_insertionMap;

public:
	Insertion_map() = default;

	~Insertion_map()
	{
		m_insertionOrder.clear();
		m_insertionOrder.shrink_to_fit();
		m_insertionMap.clear();
	}

	bool emplace_back(const varOne& valOne, const varTwo valTwo);
	std::pair<varOne, varTwo> operator[](const int index);
	std::size_t size() const { return m_insertionMap.size(); }
	bool erase(const varOne& val);
};

#endif


Insertion_map.cpp:
----------------------
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
 
#include"Insertion_map.h"
#include<algorithm>
#include<string>

/*__INSERTION_MAP__*/
/*public_function_definitions_start*/
template<typename varOne, typename varTwo>
bool Insertion_map<varOne, varTwo>::emplace_back(const varOne& valOne, const varTwo valTwo)
{//If insertion was successful .second will be true
	if (m_insertionMap.try_emplace(valOne, valTwo).second)
	{
		m_insertionOrder.emplace_back(valOne);
		return true;
	}//end of if
	else
	{
		std::cout << "Element with key (" << valOne << ") already exists.\n";
		return false;
	}//end of else
}

template<typename varOne, typename varTwo>
std::pair<varOne, varTwo> Insertion_map<varOne, varTwo>::operator[](const int index)
{ //The vector will throw an error if were out of range 
	return std::pair<varOne, varTwo>(m_insertionOrder[index],
		m_insertionMap[m_insertionOrder[index]]);
}

template<typename varOne, typename varTwo>
bool Insertion_map<varOne, varTwo>::erase(const varOne& val)
{//return true if successful and false otherwise
	if (m_insertionMap.find(val) != m_insertionMap.end())
	{
		m_insertionMap.erase(val);
		m_insertionOrder.erase(std::remove(m_insertionOrder.begin(), m_insertionOrder.end(), val),
			m_insertionOrder.end());
		return true;
	}//end of if 
	else
	{
		std::cout << "Unable to find element (" << val << ") for deletion.\n";
		return false;
	}//end of else
}
/*public_function_definitions_end*/
template class Insertion_map<std::string, void(*)()>;
/*__INSERTION_MAP__*/
globaltourist (201)
Update:
I have added some explanations, and made the code a little clearer.
I swapped the 2 vectors with 1 Insertion_map that follows the insertion order
Enoizat (1343)
The question is did i code it effectively or are there places i can optimize ?

As far as I can see, in this forum people usually ask for help on their issues.
You might also consider posting your code in a forum like this:
https://codereview.stackexchange.com/
or other similar which are reserved to give opinions about working code.
globaltourist (201)
Ok. Thanks for the advise!
TheIdeasMan (6817)
Hi,

Just a couple of things:

Try to avoid conio.h, it isn't standard - so not portable. Instead one can use the ncurses library.

https://en.wikipedia.org/wiki/Ncurses
https://www.gnu.org/software/ncurses/

Also, if you could avoid the Windows code, then that again is more portable. IIRC ncurses has a function to clear the terminal.
globaltourist (201)
Hello again TheIdeasMan!
Thanks for the feedback as always. Apart from the conio and windows code is all properly formatted and written ?
Last edited on
TheIdeasMan (6817)
Hi Daniel,

template class Insertion_map<std::string, void(*)()>;

I see that you have figured out how to separate your template functions implementations. I gather that is not a normal thing to do. What happens if the template is instantiated with something other than <std::string, void(*)()>? I notice you have explicitly specified the template parameters for the functions. Probably no real need to do that so much, the compiler can do template argument deduction and implicit instantiation.

For this:
1
2
3
 
template<typename varOne, typename varTwo>
bool Insertion_map<varOne, varTwo>::emplace_back(const varOne& valOne, const varTwo valTwo)
{


This is equivalent:

1
2
3
 
template<typename varOne, typename varTwo>
bool Insertion_map::emplace_back(const varOne& valOne, const varTwo valTwo)
{


Implicit instantiation means the types are deduced form the arguments.

http://en.cppreference.com/w/cpp/language/function_template

In Constants.cpp why all the casting? You have already specified the type, that's what they will be. Why not assign them all in the header file?

In menu.cpp , you start with a bunch of global constants (static effectively makes them global). Could you put them in a namespace, and use constexpr ? Why are they extern? Same in Console.cpp .


using funcPtr = void(*)();

This might be harder, but use std::function instead?


With this:
12
13
14
15
16
17
18
19
 
void Menu::printMenu(int curXPos, int curYPos) 
{
	Console::drawBox(m_menuHight, m_menuLength, curXPos, curYPos);
	curXPos += 2;
	curYPos += 1;
	for (int index{}; index < static_cast<int>(m_options.size()); ++index)


Some other options there:

Use std::size_t because of the size function, avoid the cast.
Ranged based for loop.

Regards :+)
Last edited on
TheIdeasMan (6817)
Another thought:

With Insertion_map , aren't you re-inventing the wheel a bit there? why not std::map<std::string, std::function>

I found this simple example, the second answer:

https://stackoverflow.com/questions/19473313/how-to-call-a-function-by-its-name-stdstring-in-c
globaltourist (201)
Insertion_map is following the order of insertion as the name states.
The regular std::map<std::string, std::function> will sort the values it gets by its key (std::string).
Now lets say i have the following functions with the following strings:

"funcA" A()
"funcB" B()
"funcC" C()

If we use the try_emplace method on the regular map in the following order:
(try_emplace in order to keep track if we insert a copy of a function with an existing key)

std::map<std::string, std::function> MAP;
MAP.try_emplace("funcB" ,B());
MAP.try_emplace("funcC" ,C());
MAP.try_emplace("funcA" ,A());

And than use a for each loop to print the elements in the map by order we would get:
"funcA" A()
"funcB" B()
"funcC" C()
Which is not the order we inserted it!

The Insertion_map uses a vector to follow the order of the keys inserted so that we can than use them in the right order which would be:
"funcB" B()
"funcC" C()
"funcA" A()
globaltourist (201)
@TheIdeasMan
Didn't see you're previous replay i'l address it now.

I see that you have figured out how to separate your template functions implementations. I gather that is not a normal thing to do. What happens if the template is instantiated with something other than <std::string, void(*)()>

The only use it should have is for <std::string, void(*)()> if i would want to implement other types i can just simply define them explicitly. And why isn't it normal ? I mean i got to this idea from stack overflow and many comments consider it a normal thing to do to separate implementation. Thats the links i used:
https://stackoverflow.com/questions/495021/why-can-templates-only-be-implemented-in-the-header-file
https://stackoverflow.com/questions/115703/storing-c-template-function-definitions-in-a-cpp-file

I notice you have explicitly specified the template parameters for the functions. Probably no real need to do that so much, the compiler can do template argument deduction and implicit instantiation.

I tried but the compiler throws error c2955 at me on the function definition line, and error c2244 at the closing brackets of the function (visual c++ 2015)

In Constants.cpp why all the casting? You have already specified the type, that's what they will be. Why not assign them all in the header file?

You have a point i removed casting, but if i assign all the values inside of the header file each time i include the header they will be instantiated and i don't want that, that's why i separated it to a cpp so that every include will use values that are instantiated only once no matter in how many other files the header is included. (this header will be reused across multiple files in the near future).

In menu.cpp , you start with a bunch of global constants (static effectively makes them global). Could you put them in a namespace, and use constexpr ? Why are they extern? Same in Console.cpp .

Will redo, but how are they extern ? static linkage doesn't allow that to happen. If you meant constants.cpp than its because the extern specifies the linkage as const is implicitly staticly linked.

This might be harder, but use std::function instead?

Should i include another library if it wont make the implementation simpler ?

Use std::size_t because of the size function, avoid the cast.

That is a better solution i will change to this.
Last edited on
globaltourist (201)
And another short question, isn't ncurses for linux only ? which basically makes it not portable as well as windows code? Because all i managed to find online is that ncurses doesn't work on windows and that i should use PDcurses instead .
TheIdeasMan (6817)
Hi,

Sorry for the delay in replying, I have been busy :+)

I see what you mean about needing a Insertion_map.

The only use it should have is for <std::string, void(*)()> if i would want to implement other types i can just simply define them explicitly. And why isn't it normal ? I mean i got to this idea from stack overflow and many comments consider it a normal thing to do to separate implementation.


One of the comments on SO was that the purpose of templates was to not have to specify a type. One can still have the functions definitions out of line, just put them and the declarations in the header file.

I tried but the compiler throws error c2955 at me ....


That was my bad, it seems one does have to specify the template parameters when the function is out of line. Perhaps another reason not to have them out of line?

You have a point i removed casting, but if i assign all the values inside of the header file each time i include the header they will be instantiated and i don't want that, that's why i separated it to a cpp so that every include will use values that are instantiated only once no matter in how many other files the header is included. (this header will be reused across multiple files in the near future).


I would have thought that is the purpose of header guards, or #pragma once (which does the same thing with a unique value). If you use constexpr they can only be defined once, at compile time.

Will redo, but how are they extern ? static linkage doesn't allow that to happen. If you meant constants.cpp than its because the extern specifies the linkage as const is implicitly staticly linked.


With the constants, I don't see the need to do any more than:

constexpr unsigned char BRAP{ 188 };

If those values are only used for the Console, then they could be members of that class. Otherwise having them in a header file is a good idea.

Should i include another library if it wont make the implementation simpler ?


Well I just think that std::function is more of a C++ way of doing things. The using funcPtr = void(*)(); looks like C: apart from using, which in that context is the same as typedef.

And another short question, isn't ncurses for linux only ? which basically makes it not portable as well as windows code?


If you used PDCurses that would still be better than conio.h IMO. I guess the syntax of PDCurses and ncurses is similar, just all the underlying stuff is different in order to get it to work on windows. So if I was using your code on my Linux machine, there would be minimal change in the code, as opposed to the conio.h code would have to be completely rewritten.

https://en.wikipedia.org/wiki/Curses_%28programming_library%29#Portability
Last edited on
globaltourist (201)
Hello!
Sorry for the delay in replying, I have been busy

No need in explanations i'm thankful that you put some of your own time to help me out.

One of the comments on SO was that the purpose of templates was to not have to specify a type. One can still have the functions definitions out of line, just put them and the declarations in the header file.

Wouldn't it be a bad practice? I mean lets say the Insertion_map had a lot more code in it, putting it all in a header would increase compile time and can cause code bloat. This is what i have in mind while the only solution is to do as i did with explicit template declaration or like this:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
 
// Foo.h
template <typename T>
struct Foo
{
    void doSomething(T param);
};

#include "Foo.tpp"

// Foo.tpp
template <typename T>
void Foo<T>::doSomething(T param)
{
    //implementation
}


I would have thought that is the purpose of header guards, or #pragma once (which does the same thing with a unique value).

I didn't mean this, i explained it poorly.

If you use constexpr they can only be defined once, at compile time.

I'l just paste a quote with a link to the source for why it wont help:
While this is simple (and fine for smaller programs), every time constants.h gets #included into a different code file, each of these variables is copied into the including code file. Therefore, if constants.h gets included into 20 different code files, each of these variables is duplicated 20 times. Header guards won’t stop this from happening, as they only prevent a header from being included more than once into a single including file, not from being included one time into multiple different code files.

http://www.learncpp.com/cpp-tutorial/42-global-variables/

With the constants, I don't see the need to do any more than:
constexpr unsigned char BRAP{ 188 };
They will be used more than just the Console.

Well I just think that std::function is more of a C++ way of doing things. The using funcPtr = void(*)(); looks like C: apart from using, which in that context is the same as typedef.

I have a pretty strange view on it, I don't see a reason to include a library whose object will increase the binary when it can be done without it and still stay readable and self explanatory. Correct me if i'm wrong here.

If you used PDCurses that would still be better than conio.h IMO. ...

Agreed but than my project would have no reason to be done as you can just use a ready implemented menu in PDcurses with much better graphics while my goal is practice.
Last edited on
TheIdeasMan (6817)
Hi,

With your templates, I have a few more thoughts:

If the template is only ever going to use <std::string, void(*)()>, then why are you using templates at all?

Wouldn't it be a bad practice? I mean lets say the Insertion_map had a lot more code in it, putting it all in a header would increase compile time and can cause code bloat. This is what i have in mind while the only solution is to do as i did with explicit template declaration or like this:


About increasing compile time: The compiler needs to have the template declaration and the implementation together (or obtain the implementation from somewhere, as you have done), in order to create code for a particular instantiation. I am not so sure that splitting the implementation away would make any difference.

About code bloat: Apparently code bloat comes from the compiler creating code for each instantiation (int, double, std::string say). Like my previous point, I don't think this has anything to do with splitting the implementation away.

Another big thing is that modern linkers are able to optimise away similar code.

With our discussions about the use of extern and constants:

So I learnt something from you with that, I had been using constexpr with an anonymous namespace, but both of those have internal linkage. So I would be better to adopt the method you are using with extern, because my constants are used in several places. Thanks for that :+)

I have a pretty strange view on it, I don't see a reason to include a library whose object will increase the binary when it can be done without it and still stay readable and self explanatory. Correct me if i'm wrong here.


Well that's Ok, but I wouldn't personally worry about adding another STL header in terms of increasing binary size - the increase is probably not worth worrying about IMO.

Agreed but than my project would have no reason to be done as you can just use a ready implemented menu in PDcurses with much better graphics while my goal is practice.


Well you could restrict yourself to the equivalent functions in PDCurses that you used in conio: gotoxy vs move say.
globaltourist (201)
So I learnt something from you with that, I had been using constexpr with an anonymous namespace, but both of those have internal linkage. So I would be better to adopt the method you are using with extern, because my constants are used in several places. Thanks for that :+)

I'm happy that i could help, you showed me a lot up to this point and i think its about time i do the same if i can XD.

And about the code itself i remade many parts of it partly reducing complexity and added some features that allow more control over the menu. Should i post it here (as a replay), edit the code above, or make a new thread entirely ?
TheIdeasMan (6817)
Should i post it here (as a replay), edit the code above, or make a new thread entirely ?


Hi,

Just post the new code here as a reply, might as well keep it altogether :+)
Pages: 123