1
\$\begingroup\$

I have wrote code for a Watch that utilizes the abstract design pattern and would like to get some feedback on the code, be as harsh as you can. Also, I would like some feedback on the singleton pattern in watch.cpp (TimeData class).

Note: I know that I could of used chrono but it wouldn't get rid of ctime and I would still need the std::tm struct for the segmented watch.

Watch.h:

#ifndef WATCH_H
#define WATCH_H

#include"Digits.h"
#include<functional>
#include<iostream>
#include<memory>
#include<ctime>

class Watch
{
public:
    virtual void printTime() = 0;
    virtual void printDate() = 0;

    void setWatchXY(int x, int y);
    void setDateXY(int x, int y);

    explicit Watch(int watchXPos, int watchYPos,
                   int dateXPos, int dateYPos)
    {
        setWatchXY(watchXPos, watchYPos);
        setDateXY(dateXPos, dateYPos);
    }

    virtual ~Watch() = default;

protected:
    int m_watchXPos;
    int m_watchYPos;
    int m_dateXPos;
    int m_dateYPos;
};

class DigitalWatch final : public Watch
{
public:
    virtual void printTime() override;
    virtual void printDate() override;

    explicit DigitalWatch(int watchXPos, int watchYPos,
                          int dateXPos, int dateYPos)
        : Watch{ watchXPos, watchYPos, dateXPos, dateYPos }
    {
    }
};

class SegmentedWatch final : public Watch
{
public:
    virtual void printTime() override;
    virtual void printDate() override;

    explicit SegmentedWatch(int watchXPos, int watchYPos,
                            int dateXPos, int dateYPos)
        : Watch{ watchXPos, watchYPos, dateXPos, dateYPos }
    {
    }

private:
    Digit m_watchDigits[4];
    Digit m_dateDigits[8];

    void printDigitAtLoc(Digit digArr[], int digit, int x, int y) const;
    void printForwardSlash(int startXPos, int startYPos);
    void printMinutes();
    void printHours();
    void printColon();
    void printMonth();
    void printYear();
    void printDay();

    void setDigitsToCurrentTime();
    void setDigitsToCurrentDate();
    void setMinutes();
    void setHours();
    void setMonth();
    void setYear();
    void setDay();
};

class Factory
{
public:
    virtual std::unique_ptr<Watch> createWatch(int watchXPos = 0, int watchYPos = 0,
                                               int dateXPos = 0, int dateYPos = 0) const = 0;
};


class DigitalWatchFactory final : public Factory
{
public:
    virtual std::unique_ptr<Watch> createWatch(int watchXPos = 0, int watchYPos = 0,
                                               int dateXPos = 0, int dateYPos = 0) const override
    {
        return std::make_unique<DigitalWatch>(watchXPos, watchYPos, dateXPos, dateYPos);
    }
};

class SegmentedWatchFactory final : public Factory
{
    virtual std::unique_ptr<Watch> createWatch(int watchXPos = 0, int watchYPos = 0,
                                               int dateXPos = 0, int dateYPos = 0) const override
    {
        return std::make_unique<SegmentedWatch>(watchXPos, watchYPos, dateXPos, dateYPos);
    }
};

#endif

Watch.cpp:

#include"Watch.h"
#include"Console.h"
#include<iomanip> //for std::put_time

namespace
{
    constexpr int yearBuffer   { 1900 };
    constexpr int colonPadding { 10 };
    constexpr int digitPadding { 5 };
    constexpr int slashHight   { 5 };
    constexpr int datePadding  { 4 };
    constexpr int slashYPadding{ 4 };
    constexpr int minutePadding{ 2 };
    constexpr int monthBuffer  { 1 };

    enum
    {
        First,
        Second,
        Third,
        Fourth,
        Fifth,
        Sixth,
        Seventh,
        Eighth
    };

    class TimeData final
    {
    public:
        static std::tm* getCurrentTime();

    private:
        static std::time_t m_rawTime;
        static std::tm     m_timeInfo;

        explicit TimeData() = default;
    };

    std::time_t TimeData::m_rawTime{};
    std::tm     TimeData::m_timeInfo{};

    std::tm* TimeData::getCurrentTime()
    {
        m_rawTime = time(NULL);
        localtime_s(&m_timeInfo, &m_rawTime);
        return &m_timeInfo;
    }
}

/*|---WATCH_FUNCTIONS_START---|*/
/*|---PUBLIC_FUNCTIONS_START---|*/
void Watch::setWatchXY(int x, int y)
{
    Console::setXY(x, y, m_watchXPos, m_watchYPos);
}

void Watch::setDateXY(int x, int y)
{
    Console::setXY(x, y, m_dateXPos, m_dateYPos);
}
/*|----PUBLIC_FUNCTIONS_END----|*/
/*|----WATCH_FUNCTIONS_END----|*/

/*|---DIGITAL_WATCH_FUNCTIONS_START---|*/
/*|---PUBLIC_FUNCTIONS_START---|*/
/*|---VIRTUAL_FUNCTIONS_START---|*/
void DigitalWatch::printTime() 
{
    Console::gotoxy(m_watchXPos, m_watchYPos);
    std::cout << std::put_time(TimeData::getCurrentTime(), "%H:%M");
}

void DigitalWatch::printDate()
{
    Console::gotoxy(m_dateXPos, m_dateYPos);
    std::cout << std::put_time(TimeData::getCurrentTime(), "%m/%d/%Y");
}
/*|----VIRTUAL_FUNCTIONS_END----|*/
/*|----PUBLIC_FUNCTIONS_END----|*/
/*|----DIGITAL_WATCH_FUNCTIONS_END----|*/

/*|---SEGMENTED_WATCH_FUNCTIONS_START---|*/
/*|---PUBLIC_FUNCTIONS_START---|*/
/*|---VIRTUAL_FUNCTIONS_START---|*/
void SegmentedWatch::printTime() 
{
    setDigitsToCurrentTime();
    printHours();
    printColon();
    printMinutes();
}

void SegmentedWatch::printDate()
{
    setDigitsToCurrentDate();
    printMonth();
    printForwardSlash(m_dateXPos + datePadding * 2 + 1,
                      m_dateYPos + slashYPadding);
    printDay();
    printForwardSlash(m_dateXPos + datePadding * 5 + 3,
                      m_dateYPos + slashYPadding);
    printYear();
}
/*|----VIRTUAL_FUNCTIONS_END----|*/
/*|----PUBLIC_FUNCTIONS_END----|*/

/*|---PRIVATE_FUNCTIONS_START---|*/
void SegmentedWatch::printDigitAtLoc(Digit digArr[], int index, int x, int y) const
{
    digArr[index].setDigitXY(x + index * digitPadding, y);
    digArr[index].printDigit();
}

void SegmentedWatch::printForwardSlash(int startXPos, int startYPos)
{
    for (int index{}; index < slashHight; ++index)
    {
        Console::putSymbol(startXPos++, startYPos--, '/');
    }//end of for
}

void SegmentedWatch::printMinutes()
{
    printDigitAtLoc(m_watchDigits, Third, m_watchXPos + minutePadding, m_watchYPos);
    printDigitAtLoc(m_watchDigits, Fourth, m_watchXPos + minutePadding, m_watchYPos);
}

void SegmentedWatch::printHours()
{
    printDigitAtLoc(m_watchDigits, First, m_watchXPos, m_watchYPos);
    printDigitAtLoc(m_watchDigits, Second, m_watchXPos, m_watchYPos);
}

void SegmentedWatch::printColon()
{
    Console::putSymbol(m_watchXPos + colonPadding, m_watchYPos + 1, '.');
    Console::putSymbol(m_watchXPos + colonPadding, m_watchYPos + 2, '.');
}

void SegmentedWatch::printMonth()
{
    printDigitAtLoc(m_dateDigits, First, m_dateXPos, m_dateYPos);
    printDigitAtLoc(m_dateDigits, Second, m_dateXPos, m_dateYPos);
}

void SegmentedWatch::printYear()
{
    printDigitAtLoc(m_dateDigits, Fifth, m_dateXPos + datePadding * 2, m_dateYPos);
    printDigitAtLoc(m_dateDigits, Sixth, m_dateXPos + datePadding * 2, m_dateYPos);
    printDigitAtLoc(m_dateDigits, Seventh, m_dateXPos + datePadding * 2, m_dateYPos);
    printDigitAtLoc(m_dateDigits, Eighth, m_dateXPos + datePadding * 2, m_dateYPos);
}

void SegmentedWatch::printDay() 
{
    printDigitAtLoc(m_dateDigits, Third, m_dateXPos + datePadding, m_dateYPos);
    printDigitAtLoc(m_dateDigits, Fourth, m_dateXPos + datePadding, m_dateYPos);
}

void SegmentedWatch::setDigitsToCurrentTime()
{
    setHours();
    setMinutes();
}

void SegmentedWatch::setDigitsToCurrentDate()
{
    setMonth();
    setDay();
    setYear();
}

void SegmentedWatch::setMinutes()
{
    m_watchDigits[Third]  = TimeData::getCurrentTime()->tm_min / 10; //First min digit
    m_watchDigits[Fourth] = TimeData::getCurrentTime()->tm_min % 10; //Second min digit
}

void SegmentedWatch::setHours()
{
    m_watchDigits[First]  = TimeData::getCurrentTime()->tm_hour / 10; //First hour digit
    m_watchDigits[Second] = TimeData::getCurrentTime()->tm_hour % 10; //Second hour digit
}

void SegmentedWatch::setMonth()
{
    m_dateDigits[First]  = TimeData::getCurrentTime()->tm_mon / 10;               //First month digit
    m_dateDigits[Second] = TimeData::getCurrentTime()->tm_mon % 10 + monthBuffer; //Second month digit
}

void SegmentedWatch::setYear()
{
    m_dateDigits[Fifth]   = (TimeData::getCurrentTime()->tm_year + yearBuffer) / 1000;                //First year digit
    m_dateDigits[Sixth]   = ((TimeData::getCurrentTime()->tm_year + yearBuffer) % 1000) / 100;        //Second year digit
    m_dateDigits[Seventh] = (((TimeData::getCurrentTime()->tm_year + yearBuffer) % 1000) % 100) / 10; //Third year digit
    m_dateDigits[Eighth]  = (((TimeData::getCurrentTime()->tm_year + yearBuffer) % 1000) % 100) % 10; //Fourth year digit
}

void SegmentedWatch::setDay()
{
    m_dateDigits[Third]  = TimeData::getCurrentTime()->tm_mday / 10; //First day digit
    m_dateDigits[Fourth] = TimeData::getCurrentTime()->tm_mday % 10; //Second day digit
}
/*|----PRIVATE_FUNCTIONS_END----|*/
/*|----SEGMENTED_WATCH_FUNCTIONS_END----|*/

The test i did in main with the watch:

#include"Watch.h"
#include"Console.h"

int main() 
{
    std::unique_ptr<Factory> segFact{ std::make_unique<SegmentedWatchFactory>() };
    std::unique_ptr<Factory> digFact{ std::make_unique<DigitalWatchFactory>() };

    std::unique_ptr<Watch> watches[2];
    watches[0] = segFact->createWatch();
    watches[1] = digFact->createWatch();

    //segmented watch
    watches[0]->setWatchXY(5, 6);
    watches[0]->setDateXY(2, 13);

    watches[0]->printTime();
    watches[0]->printDate();

    //digital watch
    //watches[1]->getWatchData().setWatchXY(5, 6);
    //watches[1]->getDateData().setDateXY(0, 0);

    //watches[1]->printTime();
    //watches[1]->printDate();
    Console::gotoxy(0, 0);
    return 0;
}

The following code is not relevant to the question, but if you would like to run the code above or use Console.h, Digits.h here is the code for 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 };
    constexpr unsigned 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::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);
}

Constants.h:

#ifndef CONSTANTS_H
#define CONSTANTS_H

namespace Shapes
{
    extern const unsigned char BRAP;
    extern const unsigned char TRAP;
    extern const unsigned char BLAP;
    extern const unsigned char TLAP;
    extern const unsigned char HP;
    extern const unsigned 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:

#include"Constants.h"

namespace Shapes
{
    extern const unsigned char BRAP{ 188 };
    extern const unsigned char TRAP{ 187 };
    extern const unsigned char BLAP{ 200 };
    extern const unsigned char TLAP{ 201 };
    extern const unsigned char HP{ 205 };
    extern const unsigned char VP{ 186 };
}

Digits.h:

#ifndef DIGITS_H
#define DIGITS_H
#include<iostream>
#include<bitset>

class Digit
{
public:
    explicit Digit(int x = 0, int y = 0)
    {
        setDigitXY(x, y);
    }

    void setDigitXY(int x, int y);
    Digit& operator=(int num);
    void printDigit();

private:
    std::bitset<10> m_segmentMask;
    int             m_digitXPos;
    int             m_digitYPos;

    void printHorizontalSegment(int firstSeg, int secondSeg);
    void printVerticalSegment(int firstSeg, int secondSeg);
};

#endif

Digits.cpp:

#include"Digits.h"
#include"Console.h"

/*|---DIGIT_FUNCTIONS_START---|*/
/*|---PUBLIC_FUNCTIONS_START---|*/
void Digit::setDigitXY(int x, int y)
{
    Console::setXY(x, y, m_digitXPos, m_digitYPos);
}

Digit& Digit::operator=(int num)
{
    if (num >= 0 && num <= 9)
    {
        switch (num)
        {
        case 0: m_segmentMask = std::bitset<10>(0x3cf); break;
        case 1: m_segmentMask = std::bitset<10>(0x88);  break;
        case 2: m_segmentMask = std::bitset<10>(0x37b); break;
        case 3: m_segmentMask = std::bitset<10>(0x3bb); break;
        case 4: m_segmentMask = std::bitset<10>(0xbc);  break;
        case 5: m_segmentMask = std::bitset<10>(0x3b7); break;
        case 6: m_segmentMask = std::bitset<10>(0x3f7); break;
        case 7: m_segmentMask = std::bitset<10>(0x8b);  break;
        case 8: m_segmentMask = std::bitset<10>(0x3ff); break;
        case 9: m_segmentMask = std::bitset<10>(0x3bf); break;
        }//end of switch
    }//end of if
    else std::cout << "Cannot assign Invalid digit must be: (0 < digit < 9).\n";
    return *this;
}

void Digit::printDigit()
{
    if (m_segmentMask.none()) std::cout << "The digit wasn't assigned a number.\n";
    else
    {
        Console::gotoxy(m_digitXPos, m_digitYPos++);
        printHorizontalSegment(0, 1);
        Console::gotoxy(m_digitXPos, m_digitYPos++);
        printVerticalSegment(2, 3);
        Console::gotoxy(m_digitXPos, m_digitYPos++);
        printHorizontalSegment(4, 5);
        Console::gotoxy(m_digitXPos, m_digitYPos++);
        printVerticalSegment(6, 7);
        Console::gotoxy(m_digitXPos, m_digitYPos);
        printHorizontalSegment(8, 9);
    }//end of else
}
/*|----PUBLIC_FUNCTIONS_END----|*/

/*|---PRIVATE_FUNCTIONS_START---|*/
void Digit::printHorizontalSegment(int firstSeg, int secondSeg)
{
    std::cout << ' ';
    if (m_segmentMask.test(firstSeg)) std::cout << '-'; else std::cout << ' ';
    if (m_segmentMask.test(secondSeg)) std::cout << '-'; else std::cout << ' ';
    std::cout << ' ';
}

void Digit::printVerticalSegment(int firstSeg, int secondSeg)
{
    if (m_segmentMask.test(firstSeg)) std::cout << '|'; else std::cout << ' ';
    std::cout << "  ";
    if (m_segmentMask.test(secondSeg)) std::cout << '|'; else std::cout << ' ';
}
/*|----PRIVATE_FUNCTIONS_END---|*/
/*|----DIGIT_FUNCTIONS_END----|*/
\$\endgroup\$
  • 1
    \$\begingroup\$ Unfortunately conio.h isn't portable c++. \$\endgroup\$ – πάντα ῥεῖ Jul 2 '17 at 14:54
  • \$\begingroup\$ Yes, i know but i need _getch() functionality and unfortunately didn't find any portable solution (I know about PDcurses but if i would use it than all of the code above will be useless as it is already implemented). \$\endgroup\$ – globalturist Jul 2 '17 at 17:40
  • \$\begingroup\$ Why use abstract factory (usually there are multiple families of objects)? I see a hierarchy of Watch (only one family). Read more at stackoverflow.com/q/13029261/1168342 \$\endgroup\$ – Fuhrmanator Jul 12 '17 at 20:43
  • \$\begingroup\$ Well you have a point here. But the code above isn't that relevant because it went through many changes(my bad for not adding the link). I'l posted a followup to it here is the link: codereview.stackexchange.com/questions/169053/… \$\endgroup\$ – globalturist Jul 13 '17 at 3:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.