Menu that utilizes the composite design pattern

Question

I have made a menu that utilizes the composite design pattern and would like to get some feedback on the code, so be as harsh as you can.

Menu.h:

#ifndef MENU_H
#define MENU_H
#include "Console.h"
#include <functional>
#include <iostream>
#include <memory>
#include <vector>
class ContainerComponent
{
public:
 virtual const std::string& getName() const = 0;
 virtual void call() = 0;
 virtual ~ContainerComponent() = default;
};
class CompositeMenu final : public ContainerComponent
{
public:
 void addChild(const std::unique_ptr<ContainerComponent> child);
 void removeChild(const std::unique_ptr<ContainerComponent> child);
 void addHeader(const std::string& header);
 void enableBoxedOptions(int boxHight = 3); 
 void disableBoxedOptions();
 void setMenuXY(int x, int y);
 virtual const std::string& getName() const override;
 virtual void call() override;
 explicit CompositeMenu(int x = 0, int y = 0, bool enabledBoxedMenu = false, int boxHight = 3)
 : m_menuXPos{ x }, m_menuYPos{ y }, m_enabledBoxedMenu{ enabledBoxedMenu }
 {//boxHight = 3 is the minimal size for symmetry
 if (m_enabledBoxedMenu)
 {
 enableBoxedOptions(boxHight);
 }//end of if
 }
private:
 std::vector<std::unique_ptr<ContainerComponent> > m_children;
 std::string m_header{};
 bool m_enabledBoxedMenu;
 bool m_enabledHeader{ false };
 int m_menuXPos;
 int m_menuYPos;
 unsigned int m_menuHight;
 unsigned int m_menuLength;
 unsigned int m_jumpRange{ 1 };
 void updateLengths(const std::string& opt, int padding);
 void printBoxedOptions(int& curXPos, int& curYPos) const;
 void printBareOptions(int& curXPos, int& curYPos) const;
 void printHeader(int& curXPos, int& curYPos) const;
 void printMenu(int curXPos, int curYPos) const;
 void executeOption(intPair coords) const;
 void updateMenuHight();
 int getPadding() const;
};
class Action final : public ContainerComponent
{
public:
 virtual const std::string& getName() const override;
 virtual void call() override;
 explicit Action(const std::string& actionName,
 const std::function<void()> action)
 : m_actionName{ actionName }, m_action{ action } 
 {
 }
private:
 const std::function<void()> m_action;
 const std::string m_actionName;
};
#endif

Menu.cpp:

#include "Menu.h"
#include <string>
#include <algorithm>
namespace
{
 unsigned constexpr char selectionArrow { 175 };
 constexpr int boxedLengthPadding{ 4 };
 constexpr int boxedHightPadding { 2 };
 constexpr int yRangePadding { 2 };
 constexpr int bareLengthPadding { 1 };
 constexpr int xJumpRange { 0 };
 int findMiddle(int num)
 {
 return ((num / 2) + 1);
 }
}
/*|---COMPOSITE_MENU_FUNCTIONS_START---|*/
/*|---PUBLIC_FUNCTIONS_START---|*/
void CompositeMenu::addChild(const std::unique_ptr<ContainerComponent> child)
{
 if (std::find_if(m_children.begin(),
 m_children.end(),
 [&child](const std::unique_ptr<ContainerComponent> cmp)
 {return cmp->getName() == child->getName(); }) == m_children.end()) 
 {
 m_children.emplace_back(child); 
 if (m_enabledBoxedMenu) updateLengths(child->getName(), boxedLengthPadding);
 else updateLengths(child->getName(), bareLengthPadding);
 }//end of if
 else std::cout << "An Action with this name already exists.\n";
}
void CompositeMenu::removeChild(const std::unique_ptr<ContainerComponent> child)
{
 if (std::find_if(m_children.begin(),
 m_children.end(),
 [&child](const std::unique_ptr<ContainerComponent> cmp)
 {return cmp.get() == child.get(); }) != m_children.end())
 {
 m_children.erase(std::remove_if(m_children.begin(),
 m_children.end(),
 [&child](const std::unique_ptr<ContainerComponent> cmp)
 {return cmp.get() == child.get(); }),
 m_children.end());
 }//end of if
 else std::cout << "The option doesn't exist.\n";
}
void CompositeMenu::addHeader(const std::string& header)
{
 m_enabledHeader = true;
 m_header = header;
 if (static_cast<unsigned int>(m_header.length()) + 1 > m_menuLength)
 {
 m_menuLength = static_cast<unsigned int>(m_header.length()) + 1;
 }//end of if
}
void CompositeMenu::enableBoxedOptions(int boxHight)
{//must be an odd number for symmetry reasons
 if ((boxHight >= 3) && (boxHight % 2 != 0))
 {
 m_enabledBoxedMenu = true;
 m_jumpRange = boxHight;
 }//end of if
 else std::cout << "Invalid box size.\n";
}
void CompositeMenu::disableBoxedOptions()
{
 if (m_enabledBoxedMenu)
 {
 m_enabledBoxedMenu = false;
 m_jumpRange = 1; //default jump range with no boxes
 updateMenuHight();
 }//end of if
}
void CompositeMenu::setMenuXY(int x, int y)
{
 Console::setXY(x, y, m_menuXPos, m_menuYPos);
}
/*|---VIRTUAL_FUNCTIONS_START---|*/
const std::string& CompositeMenu::getName() const
{
 return m_header;
}
void CompositeMenu::call() 
{
 if (static_cast<int>(m_children.size()) == 0)
 std::cout << "No options found, unable to print menu.\n";
 else
 {
 updateMenuHight();
 printMenu(m_menuXPos, m_menuYPos);
 executeOption(Console::navigation(intPair(xJumpRange, m_jumpRange),
 intPair(m_menuXPos + 1, m_menuXPos + 1),
 intPair(m_menuYPos + getPadding(),
 (m_menuYPos + getPadding()) + m_jumpRange * (static_cast<int>(m_children.size()) - 1)),
 selectionArrow));
 }//end of else
}
/*|----VIRTUAL_FUNCTIONS_END----|*/
/*|----PUBLIC_FUNCTIONS_END----|*/
/*|---PRIVATE_FUNCTIONS_START---|*/
void CompositeMenu::updateLengths(const std::string& opt, int padding)
{
 if ((static_cast<unsigned int>(opt.length()) + padding) > m_menuLength)
 {
 m_menuLength = static_cast<unsigned int>(opt.length()) + padding;
 }//end of if
}
void CompositeMenu::printBoxedOptions(int& curXPos, int& curYPos) const
{
 for (size_t index{}; index < m_children.size(); ++index)
 {
 Console::drawBox(m_jumpRange - boxedHightPadding, m_menuLength - boxedLengthPadding, curXPos, curYPos);
 Console::gotoxy(curXPos + 2, curYPos + findMiddle(m_jumpRange) - 1);
 std::cout << m_children[index]->getName();
 curYPos += m_jumpRange;
 }//end of for
}
void CompositeMenu::printBareOptions(int& curXPos, int& curYPos) const
{
 ++curXPos;
 for (size_t index{}; index < m_children.size(); ++index)
 {
 Console::gotoxy(curXPos, curYPos++);
 std::cout << m_children[index]->getName();
 }//end of for
}
void CompositeMenu::printHeader(int& curXPos, int& curYPos) const
{
 Console::gotoxy(curXPos + 1, curYPos);
 std::cout << m_header;
 Console::gotoxy(curXPos + 1, ++curYPos);
 for (size_t index{}; index < m_header.length(); ++index) std::cout << '-';
 ++curYPos;
}
void CompositeMenu::printMenu(int curXPos, int curYPos) const
{
 Console::drawBox(m_menuHight, m_menuLength, curXPos, curYPos);
 curXPos += 2;
 ++curYPos;
 if (m_enabledHeader) printHeader(curXPos, curYPos);
 if (m_enabledBoxedMenu) printBoxedOptions(curXPos, curYPos);
 else printBareOptions(curXPos, curYPos);
}
void CompositeMenu::executeOption(intPair coords) const
{
 int destX, destY;
 if (m_enabledHeader) destX = m_menuXPos + m_menuLength - 1;
 else destX = m_menuXPos + m_menuLength;
 if (m_enabledHeader) destY = m_menuYPos + m_menuHight - 3;
 else destY = m_menuYPos + m_menuHight;
 Console::clrSec(m_menuXPos, m_menuYPos, destX, destY);
 m_children[(coords.second - (m_menuYPos + getPadding())) / m_jumpRange]->call();
}
void CompositeMenu::updateMenuHight()
{
 if (m_enabledHeader) m_menuHight = m_jumpRange * static_cast<unsigned int>(m_children.size()) + 2;
 else m_menuHight = m_jumpRange * static_cast<unsigned int>(m_children.size());
}
int CompositeMenu::getPadding() const 
{
 if (m_enabledHeader) return(findMiddle(m_jumpRange) + yRangePadding);
 else return(findMiddle(m_jumpRange));
}
/*|----PRIVATE_FUNCTIONS_END----|*/
/*|----COMPOSITE_MENU_FUNCTIONS_END----|*/
/*|---ACTION_FUNCTIONS_START---|*/
/*|---PUBLIC_FUNCTIONS_START---|*/
/*|---VIRTUAL_FUNCTIONS_START---|*/
const std::string& Action::getName() const
{
 return m_actionName;
}
void Action::call() 
{
 m_action();
}
/*|----VIRTUAL_FUNCTIONS_END----|*/
/*|----PUBLIC_FUNCTIONS_END----|*/
/*|----ACTION_FUNCTIONS_END----|*/

I will also add the Console.h library that I wrote if anyone wants to use it:

Console.h:

#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 setXY(int destX, int destY, int &curX, int &curY);
 void putSymbol(int xPos, int yPos, const char symbol);
 void clrSec(int curX, int curY, int destX, int destY);
 void drawBox(int hight, int length, int x, int y);
 void gotoxy(int x, int y);
 void clrScr();
}
#endif

Console.cpp:

#include "Constants.h"
#include "Console.h" 
#include <iostream>
#include <Windows.h>
#include <conio.h>
namespace
{
 constexpr int downKey { 80 };
 constexpr int rightKey{ 77 };
 constexpr int leftKey { 75 };
 constexpr int upKey { 72 };
 unsigned constexpr char enterKey{ 13 };
 constexpr 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 };
 do {
 putSymbol(curX, curY, symbol);
 gotoxy(curX, curY);
 if (_getch() == 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);
}
void Console::setXY(int destX, int destY, int &curX, int &curY)
{
 if (destX < 0)
 {
 curX = -destX;
 std::cout << "X value was made positive.\n";
 }//end of if
 else curX = destX;
 if (destY < 0)
 {
 curY = -destY;
 std::cout << "Y value was made positive.\n";
 }//end of if
 else curY = destY;
}
inline void Console::putSymbol(int xPos, int yPos, const char symbol)
{
 gotoxy(xPos, yPos);
 std::cout << symbol;
}
void Console::clrSec(int curX, int curY, int destX, int destY)
{
 gotoxy(curX, curY);
 for (int row{}; row < destY; ++row)
 for (int col{}; col < destX; ++col)
 {
 Console::gotoxy(curX + col, curY + row);
 std::cout << ' ';
 }//end of for
 gotoxy(curX, curY);
}
void Console::drawBox(int hight, int length, int x, int y)
{
 gotoxy(x, y);
 std::cout << Shapes::topLeftAngle;
 for (int i{}; i < (length + padding); ++i) std::cout << Shapes::horizontalPiece;
 std::cout << Shapes::topRightAngle;
 for (int i{}; i < hight; ++i)
 {
 gotoxy(x, ++y);
 std::cout << Shapes::verticalPiece;
 gotoxy(x + length + 3, y); // x + length + 3 represents the furthest boarder 
 std::cout << Shapes::verticalPiece;
 }//end of for
 gotoxy(x, ++y);
 std::cout << Shapes::bottomLeftAngle;
 for (int i{}; i < (length + padding); ++i) std::cout << Shapes::horizontalPiece;
 std::cout << Shapes::bottomRightAngle;
}
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);
}

Constants.h:

#ifndef CONSTANTS_H
#define CONSTANTS_H
namespace Shapes
{
 extern unsigned const char bottomRightAngle;
 extern unsigned const char topRightAngle;
 extern unsigned const char bottomLeftAngle;
 extern unsigned const char topLeftAngle;
 extern unsigned const char horizontalPiece;
 extern unsigned const char verticalPiece;
}
#endif 

Constants.cpp:

#include "Constants.h"
namespace Shapes
{
 extern unsigned const char bottomRightAngle{ 188 };
 extern unsigned const char topRightAngle { 187 };
 extern unsigned const char bottomLeftAngle { 200 };
 extern unsigned const char topLeftAngle { 201 };
 extern unsigned const char horizontalPiece { 205 };
 extern unsigned const char verticalPiece { 186 };
}

Note that the method used for Constants.h is in place because it is meant to be included in a bunch of files and I don't want the constants to be defined in each separate file all over again.

Lastly, here is the source.cpp I used for testing:

#include "Menu.h"
#include "Console.h"
void a() { std::cout << 'a'; }
void b() { std::cout << 'b'; }
void c() { std::cout << 'c'; }
void d() { std::cout << 'd'; }
int main()
{
 std::unique_ptr<CompositeMenu> menu{ std::make_unique<CompositeMenu>(5, 5, true) };
 std::unique_ptr<CompositeMenu> submenu{ std::make_unique<CompositeMenu>(5, 5) };
 submenu->addHeader("Sub menu");
 menu->addHeader("Main menu");
 std::unique_ptr<ContainerComponent> optA{ std::make_unique<Action>("a", a) };
 std::unique_ptr<ContainerComponent> optB{ std::make_unique<Action>("b", b) };
 std::unique_ptr<ContainerComponent> optC{ std::make_unique<Action>("c", c) };
 std::unique_ptr<ContainerComponent> optD{ std::make_unique<Action>("d", d) };
 menu->addChild(std::move(optA));
 menu->addChild(std::move(optB));
 menu->addChild(std::move(optC));
 menu->addChild(std::move(optD));
 menu->removeChild(std::move(optB));
 submenu->addChild(std::move(optA));
 submenu->addChild(std::move(optB));
 submenu->addChild(std::move(optC));
 submenu->addChild(std::move(optD));
 menu->addChild(std::move(submenu));
 menu->call();
 return 0;
}

• Ben Steffan: Choose const unsigned char over unsigned const char. The const is likely to be overlooked
• Ben Steffan: Also, I don't think using intPair = std::pair<int, int>; actually helps readability. I would go without it.

Answer 1

Just a few hints I gathered:

1. const should be the first modifier in most cases. Things like unsigned const char are bad because most people are used to thinking of it as an unsigned char which is also const, in contrast to a const char which is also unsigned (i.e. the first level of categorization is usually unsigned, then const).
2. Leave a space between #include and the corresponding angle brackets/quotation marks.
3. The fact that Constants.h has a "meanings" comment hints you at the fact that you should probably choose better names for your variables. Usually, typing a few more characters is not going to hurt your coding performance very much (especially not in times of auto completion!) and will make you code much more readable. As a reviewer, I do not want to switch back to Constants.h to look up the meaning of a variable every time I encounter one.
4. CompositeMenu::addChild should probably take its parameter by const reference, because you are not actually doing any modifications to the object.
5. You should rethink your approach on ownership. Currently, CompositeMenu does not own its children, which means that they have to be owned by something else. That means that your whole design structure is not self-sufficient; you always need some kind of class that maintains a vector of all current ContainerComponents, to which each of your CompositeMenus only holds a reference. You also need to make sure that none of the objects expire before your CompositeMenu, or else you will run into undefined behavior. Also, if any of your ContainerComponents are moved, you will need to update their reference accordingly in their CompositeMenu, which is cumbersome at best and invokes undefined behavior at worst if you make a mistake. Thus, I suggest you to change the relationship into an owning one, and possible give out references or pointers to other users (also, you still need to face the issue of relocation and thus invalidation, which is why you will probably need another layer of indirection (i.e. a vector of ContainerComponent*))
6. If you want to begin your class definitions with the public members, I would recommend you to use struct instead, since all members of struct are implicitly public unless marked otherwise (still, this is very much a question of personal style)
7. Use the correct integer types. For example, it appears that m_menuLength can never be lower than 0, so it should at the very least be unsigned. However, for variables denoting a length, std::size_t is generally more appropriate (especially since you assign it the length of a string, which is also of type std::size_t).

• Mathieu Guindon: Comments are not for extended discussion; this conversation has been moved to chat.