4
\$\begingroup\$

This is a follow up question to Inserting and retrieving data MySql Database access in C++ using Boost::ASIO and Boost::MySQL. I have reduced the amount of code to be reviewed by presenting only one of the models being tested by the unit test, and non of the unit testing code. I would really like this code reviewed, and I will be posting a bounty.

This question is also related to C++ Date Model to be used within models in an MVVM application. The dates in the Task Model presented below are what I was trying to implement in that question. I moved from Ubuntu 22.04 to 24.04 to get the necessary changes in g++-14 (Ubuntu 22.04 was limited to g++-12).

The program is testing the database interface for a project planning and personal scheduling system I am planning on developing.

All of the necessary tables are created by PlannerTaskScheduleDB.sql, only 2 of the models are currently implemented and being tested, but they are the 2 most complex models.

New Design

The models and database interface have been completely restructured from the previous version. The previous version attempted to keep the models and the database interface separate, in this version the model contains the database interface. This allowed a lot of the boiler plate database access code to be reduced.

Why ListDBInterface.h? It didn't seem proper for a model to return a list of that model.

My Concerns

  1. Is the code maintainable? Could you take over coding this project, or part of this project?
  2. Are the object partitioned well? Are there any dependencies between objects that could be a problem?
  3. Performance, the primary concern is run time efficiency. The unit tests runs in .5 seconds, the valgrind test takes about 6 seconds.
  4. DRY code, is there any code that repeats that can be reduced.
  5. Code Complexity.
  6. Code reduction, each model will add a minimum of 300 lines of code and up to a 1000 lines of code, are there any modern C++ features I can use to reduce this?
  7. Is any part of this a bad object oriented design?
  8. ListDBInterface.h is only my second attempt at writing a template class, what are the problems with it?

Test run using valgrind

Memcheck, a memory error detector
Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
Using Valgrind-3.22.0 and LibVEX; rerun with -h for copyright info


All positive path tests for database insertions and retrievals of user PASSED!
All negative path tests for database insertions and retrievals of user PASSED!
All tests for database insertions and retrievals of user PASSED!
All positive path tests for database insertions and retrievals of task PASSED!
All negative path tests for database insertions and retrievals of task PASSED!
All tests for database insertions and retrievals of task PASSED!
All tests Passed

HEAP SUMMARY:
    in use at exit: 0 bytes in 0 blocks
  total heap usage: 34,823 allocs, 34,823 frees, 4,404,863 bytes allocated

All heap blocks were freed -- no leaks are possible

For lists of detected and suppressed errors, rerun with: -s
ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 112 from 13)

The suppressed errors are checks for valid data in the model before inserting the model data into the database, they are caused by checking std::chrono::year_day_model::ok(() on uninitialized std::chrono::year_day_model variables in the TaskModel.

Development Environment

  • Ubuntu 24.04
  • g++ 14
  • CMake 4.01
  • Boost 1.88
  • MariaDB Ver 15.1 Distrib 10.11.13-MariaDB
  • Visual Studio Code

The Code

The GitHub repository contains the entire program including the other models being tested. This repository was created specifically for this code review question. The actual development repository.

If you are running Ubuntu 24.04 with g++14 you can build it. If you have MariaDB or MySQL installed you should be able to run the program if you so desire.

Shell file that runs the unit test

After every edit I run the following to perform regression testing prior to checking the files into git.

#!/bin/bash
mysql -u MySQLUser -p < PlannerTaskScheduleDB.sql
protoPersonalPlanner -u MySQLUser -p MySQLPassword >& UnitTests/testOut.txt
mysql -u MySQLUser -p < PlannerTaskScheduleDB.sql
valgrind --track-origins=yes --suppressions=UnitTests/unitTestValgrindSuppressExpectedErrors.supp  protoPersonalPlanner -u MySQLUser -p MySQLPassword  2>&1 | sed 's/^==[0-9]*== //' > UnitTests/valgrindOut.txt
echo "Diff"
diff UnitTests/testOut.txt UnitTests/testOut_forDiff.txt
echo "valgrind Diff"
diff UnitTests/valgrindOut.txt UnitTests/valgrindOut_forDiff.txt

paulc> wc Core* List* Model* Task*
116 187 2825 CoreDBInterface.cpp
42 93 1233 CoreDBInterface.h
99 303 3030 ListDBInterface.h
203 351 4317 ModelDBInterface.cpp
118 313 3804 ModelDBInterface.h
125 186 3000 TaskList.cpp
31 55 837 TaskList.h
561 1200 17511 TaskModel.cpp
229 841 10450 TaskModel.h
1524 3529 47007 total

CoreDBInterface.h

#ifndef COREDBINTERFACECORE_H_
#define COREDBINTERFACECORE_H_

#include <boost/asio.hpp>
#include <boost/mysql.hpp>
#include "CommandLineParser.h"
#include <iostream>
#include <optional>
#include <string>

namespace NSBA = boost::asio;
namespace NSBM = boost::mysql;

class CoreDBInterface
{
public:
    CoreDBInterface();
    virtual ~CoreDBInterface() = default;
    std::string getAllErrorMessages() const noexcept { return errorMessages; };

protected:
    void initFormatOptions();
    void prepareForRunQueryAsync();
    void appendErrorMessage(const std::string& newError) { errorMessages.append(newError); errorMessages.append("\n");};
/*
 * All calls to runQueryAsync and getConnectionFormatOptsAsync should be
 * implemented within try blocks.
 */
    NSBM::results runQueryAsync(const std::string& query);
    NSBM::format_options getConnectionFormatOptsAsync();
    NSBA::awaitable<NSBM::results> coRoutineExecuteSqlStatement(const std::string& query);
    NSBA::awaitable<NSBM::format_options> coRoutineGetFormatOptions();

    std::string errorMessages;
    NSBM::connect_params dbConnectionParameters;
    bool verboseOutput;
    std::optional<NSBM::format_options> format_opts;
};

#endif // COREDBINTERFACECORE_H_

CoreDBInterface.cpp

#include <boost/asio.hpp>
#include <boost/mysql.hpp>
#include "CommandLineParser.h"
#include "CoreDBInterface.h"
#include <iostream>
#include <string>

CoreDBInterface::CoreDBInterface()
:   errorMessages{""},
    verboseOutput{programOptions.verboseOutput}
{
    dbConnectionParameters.server_address.emplace_host_and_port(programOptions.mySqlUrl, programOptions.mySqlPort);
    dbConnectionParameters.username = programOptions.mySqlUser;
    dbConnectionParameters.password = programOptions.mySqlPassword;
    dbConnectionParameters.database = programOptions.mySqlDBName;
}

void CoreDBInterface::initFormatOptions()
{
    try {
        if (!format_opts.has_value())
        {
            format_opts = getConnectionFormatOptsAsync();
        }
    }
    catch (const std::exception& e)
    {
        std::cerr << "ERROR: initFormatOptions() FAILED: " << e.what() << "\n";
    }
}

void CoreDBInterface::prepareForRunQueryAsync()
{
    errorMessages.clear();
    initFormatOptions();
};

/*
 * All calls to runQueryAsync should be implemented within try blocks.
 */
NSBM::results CoreDBInterface::runQueryAsync(const std::string& query)
{
    NSBM::results localResult;
    NSBA::io_context ctx;

    NSBA::co_spawn(ctx, coRoutineExecuteSqlStatement(query),
        [&localResult, this](std::exception_ptr ptr, NSBM::results result)
        {
            if (ptr)
            {
                std::rethrow_exception(ptr);
            }
            localResult = std::move(result);
        }
    );

    ctx.run();

    return localResult;
}

NSBA::awaitable<NSBM::results> CoreDBInterface::coRoutineExecuteSqlStatement(const std::string& query)
{
    NSBM::any_connection conn(co_await NSBA::this_coro::executor);

    co_await conn.async_connect(dbConnectionParameters);
    
    NSBM::results selectResult;

    if (verboseOutput)
    {
        std::clog << "Running: \n\t" << query << "\n";
    }

    co_await conn.async_execute(query, selectResult);

    co_await conn.async_close();

    co_return selectResult;
}

NSBM::format_options CoreDBInterface::getConnectionFormatOptsAsync()
{
    NSBM::format_options options;
    NSBA::io_context ctx;

    NSBA::co_spawn(ctx, coRoutineGetFormatOptions(),
        [&options, this](std::exception_ptr ptr, NSBM::format_options result)
        {
            if (ptr)
            {
                std::rethrow_exception(ptr);
            }
            options = std::move(result);
        }
    );

    ctx.run();

    return options;
}

NSBA::awaitable<NSBM::format_options> CoreDBInterface::coRoutineGetFormatOptions()
{
    NSBM::any_connection conn(co_await NSBA::this_coro::executor);

    co_await conn.async_connect(dbConnectionParameters);

    NSBM::format_options options = conn.format_opts().value();

    co_await conn.async_close();

    co_return options;
}

ListDBInterface.h

#ifndef LISTDBINTERFACECORE_H_
#define LISTDBINTERFACECORE_H_

#include <boost/asio.hpp>
#include <boost/mysql.hpp>
#include <concepts>
#include "CoreDBInterface.h"
#include <iostream>
#include <memory>
#include "ModelDBInterface.h"
#include <string>
#include <string_view>
#include <vector>

/*
 * Templated Class to handle model select queries with multiple results.
 * 
 * The model itself is used to create the select statement. The first query will
 * only return the primary key of each instance of the model that matches the
 * search parameters of the query. Once the list of primary keys is established
 * the model will be used to select the object by primary key for the full data.
 * 
 * This is done primarily to keep the table related information for a model in
 * the model. A second reason for this design is that the is a maximum size that
 * a boost::mysql::results class can reach; to reduce the possiblity of reaching
 * that maximum queries that can return a large amount of data will be reduced
 * to returning only the primary keys of the data.
 * 
 * This design may be revisited if there is too much a performance degradation.
 */
template<typename ListType>
requires std::is_base_of<ModelDBInterface, ListType>::value
class ListDBInterface : public CoreDBInterface
{
public:
    ListDBInterface()
    : CoreDBInterface()
    {
        std::string tempListType(queryGenerator.getModelName());
        tempListType += "List";
        listTypeName = tempListType;
    }
    virtual ~ListDBInterface() = default;

    std::string_view getListTypeName() const noexcept { return listTypeName; };
    
    bool runFirstQuery()
    {
        prepareForRunQueryAsync();

        try
        {
            primaryKeyResults.clear();
            NSBM::results localResult = runQueryAsync(firstFormattedQuery);
            primaryKeyResults = processFirstQueryResults(localResult);

            return primaryKeyResults.size() > 0;
        }

        catch(const std::exception& e)
        {
            appendErrorMessage(std::format("In {}List.runFirstQuery() : {}",
                queryGenerator.getModelName(), e.what()));
            return false;
        }
    };

protected:
    void setFirstQuery(std::string formattedQueryStatement) noexcept
    {
        firstFormattedQuery = formattedQueryStatement;
    }
    
    virtual std::vector<std::size_t> processFirstQueryResults(NSBM::results& results)
    {
        if (results.rows().empty())
        {
            appendErrorMessage(std::format("No {}s found!", queryGenerator.getModelName()));
            return primaryKeyResults;
        }

        for (auto row: results.rows())
        {
            primaryKeyResults.push_back(row.at(0).as_uint64());
    
        }
        return primaryKeyResults;
    }

    ListType queryGenerator;
    std::string_view listTypeName;
    std::string firstFormattedQuery;
    std::vector<std::size_t> primaryKeyResults;
    std::vector<std::shared_ptr<ListType>> returnType;
};

#endif // LISTDBINTERFACECORE_H_

ModelDBInterface.h

#ifndef MODELDBINTERFACECORE_H_
#define MODELDBINTERFACECORE_H_

#include <boost/asio.hpp>
#include <boost/mysql.hpp>
#include <chrono>
#include "CoreDBInterface.h"
#include <functional>
#include <iostream>
#include <optional>
#include <string>
#include <string_view>
#include <vector>

class ModelDBInterface : public CoreDBInterface
{
public:
    ModelDBInterface(std::string_view modelNameIn);
    virtual ~ModelDBInterface() = default;
    bool save();
    bool insert();
    bool update();
    bool retrieve();    // Only select object by object ID.
    bool isInDataBase() const noexcept { return (primaryKey > 0); };
    bool isModified() const noexcept { return modified; };
    void clearModified() { modified = false; };
    bool hasRequiredValues();
    void reportMissingFields() noexcept;
    std::string_view getModelName() const { return modelName; };

protected:
/*
 * Each model will have 1 or more required fields, the model must specify what those
 * fields are.
 */
    virtual void initRequiredFields() = 0;
/*
 * Each model must provide formating for Insert, Update and Select by object ID.
 * Additional select statements will be handled by each model as necessary.
 */
    virtual std::string formatInsertStatement() = 0;
    virtual std::string formatUpdateStatement() = 0;
    virtual std::string formatSelectStatement() = 0;
    virtual bool processResult(NSBM::results& results);
/*
 * Each model must provide the process by which the database information will
 * be translated into the specific model.
 */
    virtual void processResultRow(NSBM::row_view rv) = 0;

/*
 * To process TEXT fields that contain model fields.
 */
    std::vector<std::string> explodeTextField(std::string const& textField) noexcept;
    std::string implodeTextField(std::vector<std::string>& fields) noexcept;

    NSBM::date stdchronoDateToBoostMySQLDate(const std::chrono::year_month_day& source) noexcept
    {
        std::chrono::sys_days tp = source;
        return NSBM::date(tp);
    };

    std::chrono::year_month_day boostMysqlDateToChronoDate(const NSBM::date& source) noexcept
    {
        const std::chrono::year year{source.year()};
        const std::chrono::month month{source.month()};
        const std::chrono::day day{source.day()};
        return std::chrono::year_month_day{year, month, day};
    };

    NSBM::datetime stdChronoTimePointToBoostDateTime(std::chrono::system_clock::time_point source) noexcept
    {
        return NSBM::datetime(std::chrono::time_point_cast<boost::mysql::datetime::time_point::duration>(source));
    };

    std::chrono::system_clock::time_point boostMysqlDateTimeToChronoTimePoint(NSBM::datetime dbDateTime)
    {
        return std::chrono::time_point_cast<std::chrono::system_clock::time_point::duration>(dbDateTime.as_time_point());
    }

    std::optional<NSBM::date> optionalDateConversion(std::optional<std::chrono::year_month_day> optDate)
    {
        std::optional<NSBM::date> mySqlDate;

        if (optDate.has_value())
        {
            mySqlDate = stdchronoDateToBoostMySQLDate(optDate.value());
        }
        return mySqlDate;
    };

    std::optional<NSBM::datetime> optionalDateTimeConversion(std::optional<std::chrono::system_clock::time_point> optDateTime)
    {
        std::optional<NSBM::datetime> timeStamp;

        if (optDateTime.has_value())
        {
            timeStamp = stdChronoTimePointToBoostDateTime(optDateTime.value());
        }
        return timeStamp;
    };

protected:
    std::size_t primaryKey;
    std::string_view modelName;
    bool modified;
    char delimiter;
    struct RequireField
    {
        std::function<bool(void)>errorCondition;
        std::string fieldName;
    };
    std::vector<RequireField> missingRequiredFieldsTests;
};

#endif // MODELDBINTERFACECORE_H_

ModelDBInterface.cpp

#include <boost/asio.hpp>
#include <boost/mysql.hpp>
#include <chrono>
#include "ModelDBInterface.h"
#include <iostream>
#include <sstream>
#include <string>
#include <string_view>
#include <vector>

ModelDBInterface::ModelDBInterface(std::string_view modelNameIn)
: CoreDBInterface()
{
    primaryKey = 0;
    modelName = modelNameIn;
    modified = false;
    delimiter = ';';  
}

bool ModelDBInterface::save()
{
    if (!isModified())
    {
        appendErrorMessage(std::format("{} not modified, no changes to save", modelName));
        return false;
    }

    if (isInDataBase())
    {
        return update();
    }
    else
    {
        return insert();
    }
}

bool ModelDBInterface::insert()
{
    errorMessages.clear();

    if (isInDataBase())
    {
        appendErrorMessage(std::format("{} already in Database, use Update!", modelName));
        return false;
    }

    if (!isModified())
    {
        appendErrorMessage(std::format("{} not modified!", modelName));
        return false;
    }

    if (!hasRequiredValues())
    {
        appendErrorMessage(std::format("{} is missing required values!", modelName));
        reportMissingFields();
        return false;
    }

    prepareForRunQueryAsync();

    try
    {
        NSBM::results localResult = runQueryAsync(formatInsertStatement());
        primaryKey = localResult.last_insert_id();
        modified = false;
    }

    catch(const std::exception& e)
    {
        appendErrorMessage(std::format("In {}.insert : {}", modelName, e.what()));
        return false;
    }

    return true;
}

bool ModelDBInterface::update()
{
    prepareForRunQueryAsync();

    if (!isInDataBase())
    {
        appendErrorMessage(std::format("{} not in Database, use Insert!", modelName));
        return false;
    }

    if (!isModified())
    {
        appendErrorMessage(std::format("{} not modified!", modelName));
        return false;
    }

    try
    {

        NSBM::results localResult = runQueryAsync(formatUpdateStatement());
        modified = false;
            
        return true;
    }

    catch(const std::exception& e)
    {
        appendErrorMessage(std::format("In {}.update : {}", modelName, e.what()));
        return false;
    }
}

bool ModelDBInterface::retrieve()
{
    prepareForRunQueryAsync();

    try
    {
        NSBM::results localResult = runQueryAsync(formatSelectStatement());

        return processResult(localResult);
    }

    catch(const std::exception& e)
    {
        appendErrorMessage(std::format("In {}.retrieve() : {}", modelName, e.what()));
        return false;
    }
}

bool ModelDBInterface::hasRequiredValues()
{
    initRequiredFields();
    for (auto fieldTest: missingRequiredFieldsTests)
    {
        if (fieldTest.errorCondition())
        {
            return false;
        }
    }
    
    return true;
}

/*
 * Assumes that ModelDBInterface::hasRequiredValues() was called previously and that
 * initRequiredFields() has been called.
 */
void ModelDBInterface::reportMissingFields() noexcept
{
    for (auto testAndReport: missingRequiredFieldsTests)
    {
        if (testAndReport.errorCondition())
        {
            appendErrorMessage(std::format("Missing {} required {}!\n", modelName, testAndReport.fieldName));
        }
    }
}

bool ModelDBInterface::processResult(NSBM::results& results)
{
    if (results.rows().empty())
    {
        appendErrorMessage(std::format("{} not found!", modelName));
        return false;
    }

    if (results.rows().size() > 1)
    {
        appendErrorMessage(std::format("Too many {}s found to process!", modelName));
        return false;
    }

    NSBM::row_view rv = results.rows().at(0);
    processResultRow(rv);

    return true;
}

std::vector<std::string> ModelDBInterface::explodeTextField(std::string const& textField) noexcept
{
    std::vector<std::string> subFields;
    std::istringstream iss(textField);

    for (std::string token; std::getline(iss, token, delimiter); )
    {
        subFields.push_back(std::move(token));
    }
    return subFields;
}

std::string ModelDBInterface::implodeTextField(std::vector<std::string> &fields) noexcept
{
    std::string textField;

    for (auto field: fields)
    {
        textField.append(field);
        textField += delimiter;
    }

    return textField;
}

TaskModel.h

#ifndef TASKMODEL_H_
#define TASKMODEL_H_

#include <chrono>
#include "commonUtilities.h"
#include <format>
#include <functional>
#include <iostream>
#include <memory>
#include "ModelDBInterface.h"
#include <optional>
#include <string>
#include <vector>

class TaskModel : public ModelDBInterface
{
public:
    enum class TaskStatus
    {
        Not_Started, On_Hold, Waiting_for_Dependency, Work_in_Progress, Complete
    };

    TaskModel();
    TaskModel(std::size_t creatorID);
    TaskModel(std::size_t creatorID, std::string descriptionIn);
    virtual ~TaskModel() = default;

    void addEffortHours(double hours);
    void markComplete()
    {
        setCompletionDate(getTodaysDate());
        setStatus(TaskModel::TaskStatus::Complete);
    }
    std::size_t getTaskID() const { return primaryKey; };
    std::size_t getCreatorID() const { return creatorID; };
    std::size_t getAssignToID() const { return assignToID; };
    std::string getDescription() const { return description; };
    TaskModel::TaskStatus getStatus() const { return status.value_or(TaskModel::TaskStatus::Not_Started); };
    unsigned int getStatusIntVal() const { return static_cast<unsigned int>(getStatus()); };
    std::size_t getParentTaskID() const { return parentTaskID.value_or(0); };
    std::optional<std::size_t> rawParentTaskID() const { return parentTaskID; };
    double getPercentageComplete() const { return percentageComplete; };
    std::chrono::year_month_day getCreationDate() const { return creationDate; };
    std::chrono::year_month_day getDueDate() const { return dueDate; };
    std::chrono::year_month_day getScheduledStart() const { return scheduledStart; };
    std::chrono::year_month_day getactualStartDate() const;
    std::optional<std::chrono::year_month_day> rawActualStartDate() const { return actualStartDate; };
    std::chrono::year_month_day getEstimatedCompletion() const;
    std::optional<std::chrono::year_month_day> rawEstimatedCompletion() const { return estimatedCompletion; };
    std::chrono::year_month_day getCompletionDate() const ;
    std::optional<std::chrono::year_month_day> rawCompletionDate() const { return completionDate; };
    unsigned int getEstimatedEffort() const { return estimatedEffort; };
    double getactualEffortToDate() const { return actualEffortToDate; };
    unsigned int getPriorityGroup() const { return priorityGroup; };
    unsigned int getPriority() const { return priority; };
    std::vector<std::size_t> getDependencies() { return dependencies; };
    bool isPersonal() const { return personal; };
    void setCreatorID(std::size_t creatorID);
    void setAssignToID(std::size_t assignedID);
    void setDescription(std::string description);
    void setStatus(TaskModel::TaskStatus status);
    void setStatus(std::string statusStr) { setStatus(stringToStatus(statusStr)); };
    void setParentTaskID(std::size_t parentTaskID);
    void setParentTaskID(std::shared_ptr<TaskModel> parentTask) { setParentTaskID(parentTask->getTaskID()); };
    void setPercentageComplete(double percentComplete);
    void setCreationDate(std::chrono::year_month_day creationDate);
    void setDueDate(std::chrono::year_month_day dueDate);
    void setScheduledStart(std::chrono::year_month_day startDate);
    void setactualStartDate(std::chrono::year_month_day startDate);
    void setEstimatedCompletion(std::chrono::year_month_day completionDate);
    void setCompletionDate(std::chrono::year_month_day completionDate);
    void setEstimatedEffort(unsigned int estimatedHours);
    void setActualEffortToDate(double effortHoursYTD);
    void setPriorityGroup(unsigned int priorityGroup);
    void setPriorityGroupC(const char priorityGroup);
    void setPriority(unsigned int priority);
    void setPersonal(bool personalIn);
    void addDependency(std::size_t taskId);
    void addDependency(TaskModel& dependency) { addDependency(dependency.getTaskID()); };
    void addDependency(std::shared_ptr<TaskModel> dependency) { addDependency(dependency->getTaskID()); };
    void setTaskID(std::size_t newID);
    std::string taskStatusString() const;
    TaskModel::TaskStatus stringToStatus(std::string statusName) const;
/*
 * Select with arguments
 */
    bool selectByDescriptionAndAssignedUser(std::string_view description, std::size_t assignedUserID);
    bool selectByTaskID(std::size_t taskID);
    // Return multiple Tasks.
    std::string formatSelectActiveTasksForAssignedUser(std::size_t assignedUserID);
    std::string formatSelectUnstartedDueForStartForAssignedUser(std::size_t assignedUserID);
    std::string formatSelectTasksCompletedByAssignedAfterDate(std::size_t assignedUserID,
        std::chrono::year_month_day& searchStartDate);
    std::string formatSelectTasksByAssignedIDandParentID(std::size_t assignedUserID, std::size_t parentID);

/*
 * Required fields.
 */
    bool isMissingDescription() { return (description.empty() || description.length() < MinimumDescriptionLength); };
    bool isMissingCreatorID() { return creatorID == 0; };
    bool isMissingAssignedID() { return assignToID == 0; };
    bool isMissingEffortEstimate() { return estimatedEffort == 0; };
    bool isMissingPriorityGroup() { return priorityGroup == 0; };
    bool isMissingCreationDate() { return !creationDate.ok(); };
    bool isMissingScheduledStart() { return !scheduledStart.ok(); };
    bool isMissingDueDate() { return !dueDate.ok(); };


    bool operator==(TaskModel& other)
    {
        return diffTask(other);
    }
    bool operator==(std::shared_ptr<TaskModel> other)
    {
        return diffTask(*other);
    }

    friend std::ostream& operator<<(std::ostream& os, const TaskModel& task)
    {
        constexpr const char* outFmtStr = "\t{}: {}\n";
        os << "TaskModel:\n";
        os << std::format(outFmtStr, "Task ID", task.primaryKey);
        os << std::format(outFmtStr, "Creator ID", task.creatorID);
        os << std::format(outFmtStr, "Assigned To ID", task.assignToID);
        os << std::format(outFmtStr, "Description", task.description);
        os << std::format(outFmtStr, "Percentage Complete", task.percentageComplete);
        os << std::format(outFmtStr, "Creation Date", task.creationDate);
        os << std::format(outFmtStr, "Scheduled Start Date", task.scheduledStart);
        os << std::format(outFmtStr, "Due Date", task.dueDate);
        os << std::format(outFmtStr, "Estimated Effort Hours", task.estimatedEffort);
        os << std::format(outFmtStr, "Actual Effort Hours", task.actualEffortToDate);
        os << std::format(outFmtStr, "Priority Group", task.priorityGroup);
        os << std::format(outFmtStr, "Priority", task.priority);

        os << "Optional Fields\n";
        if (task.status.has_value())
        {
            os << std::format(outFmtStr, "Status", task.taskStatusString());
        }
        if (task.parentTaskID.has_value())
        {
            os << std::format(outFmtStr, "Parent ID", task.parentTaskID.value());
        }
        if (task.actualStartDate.has_value())
        {
            os << std::format(outFmtStr, "Actual Start Date", task.actualStartDate.value());
        }
        if (task.estimatedCompletion.has_value())
        {
            os << std::format(outFmtStr, "Estimated Completion Date", task.estimatedCompletion.value());
        }
        if (task.completionDate.has_value())
        {
            os << std::format(outFmtStr, "Completed Date", task.completionDate.value());
        }

        return os;
    };


private:
    TaskStatus statusFromInt(unsigned int statusI) const { return static_cast<TaskModel::TaskStatus>(statusI); };
    bool diffTask(TaskModel& other);
    std::string formatInsertStatement() override;
    std::string formatUpdateStatement() override;
    std::string formatSelectStatement() override;
    void initRequiredFields() override;
    void addDependencies(const std::string& dependenciesText);
    std::string buildDependenciesText(std::vector<std::size_t>& dependencyList) noexcept;
    void processResultRow(NSBM::row_view rv) override;

    std::size_t creatorID;
    std::size_t assignToID;
    std::string description;
    std::optional<TaskStatus> status;
    std::optional<std::size_t> parentTaskID;
    double percentageComplete;
    std::chrono::year_month_day creationDate;
    std::chrono::year_month_day dueDate;
    std::chrono::year_month_day scheduledStart;
    std::optional<std::chrono::year_month_day> actualStartDate;
    std::optional<std::chrono::year_month_day> estimatedCompletion;
    std::optional<std::chrono::year_month_day> completionDate;
    unsigned int estimatedEffort;
    double actualEffortToDate;
    unsigned int priorityGroup;
    unsigned int priority;
    bool personal;
    const std::size_t MinimumDescriptionLength = 10;
    std::vector<std::size_t> dependencies;

/*
 * The indexes below are based on the following select statement, maintain this order
 * baseQuery could be SELECT * FROM Tasks, but this way the order of the columns
 * returned are known.
 */
    NSBM::constant_string_view baseQuery = "SELECT TaskID, CreatedBy, AsignedTo, Description, ParentTask, Status, PercentageComplete, CreatedOn,"
            "RequiredDelivery, ScheduledStart, ActualStart, EstimatedCompletion, Completed, EstimatedEffortHours, "
            "ActualEffortHours, SchedulePriorityGroup, PriorityInGroup, Personal, DependencyCount, Dependencies FROM Tasks ";

    const std::size_t taskIdIdx = 0;
    const std::size_t createdByIdx = 1;
    const std::size_t assignedToIdx = 2;
    const std::size_t descriptionIdx = 3;
    const std::size_t parentTaskIdx = 4;
    const std::size_t statusIdx = 5;
    const std::size_t percentageCompleteIdx = 6;
    const std::size_t createdOnIdx = 7;
    const std::size_t requiredDeliveryIdx = 8;
    const std::size_t scheduledStartIdx = 9;
    const std::size_t actualStartIdx = 10;
    const std::size_t estimatedCompletionIdx = 11;
    const std::size_t completedIdx = 12;
    const std::size_t estimatedEffortHoursIdx = 13;
    const std::size_t actualEffortHoursIdx = 14;
    const std::size_t schedulePriorityGroupIdx = 15;
    const std::size_t priorityInGroupIdx = 16;
    const std::size_t personalIdx = 17;
    const std::size_t dependencyCountIdx = 18;
    const std::size_t depenedenciesTextIdx = 19;

    NSBM::constant_string_view listQueryBase = "SELECT TaskID FROM Tasks ";
};

using TaskModel_shp = std::shared_ptr<TaskModel>;

#endif // TASKMODEL_H_

TaskModel.cpp

#include <chrono>
#include "commonUtilities.h"
#include <functional>
#include "GenericDictionary.h"
#include <iostream>
#include <memory>
#include <string>
#include "TaskModel.h"
//#include "UserModel.h"
#include <vector>

static const TaskModel::TaskStatus UnknowStatus = static_cast<TaskModel::TaskStatus>(-1);

static std::vector<GenericDictionary<TaskModel::TaskStatus, std::string>::DictType> statusConversionsDefs = {
    {TaskModel::TaskStatus::Not_Started, "Not Started"},
    {TaskModel::TaskStatus::On_Hold, "On Hold"},
    {TaskModel::TaskStatus::Waiting_for_Dependency, "Waiting for Dependency"},
    {TaskModel::TaskStatus::Work_in_Progress, "Work in Progress"},
    {TaskModel::TaskStatus::Complete, "Completed"}
};

static GenericDictionary<TaskModel::TaskStatus, std::string> taskStatusConversionTable(statusConversionsDefs);

TaskModel::TaskModel()
: ModelDBInterface("Task")
{
  creatorID = 0;
  assignToID = 0;
  description = "";
  percentageComplete = 0.0;
  estimatedEffort = 0;
  actualEffortToDate = 0.0;
  priorityGroup = 0;
  priority = 0;
  personal = false;
}

TaskModel::TaskModel(std::size_t creatorID)
: TaskModel()
{
    setCreatorID(creatorID);
    setAssignToID(creatorID);
}

TaskModel::TaskModel(std::size_t creatorID, std::string description)
: TaskModel()
{
    setCreatorID(creatorID);
    setAssignToID(creatorID);
    setDescription(description);
}

void TaskModel::addEffortHours(double hours)
{
    double actualEffortHours = getactualEffortToDate();
    actualEffortHours += hours;
    setActualEffortToDate(actualEffortHours);
}

std::chrono::year_month_day TaskModel::getactualStartDate() const
{
    return actualStartDate.value_or(std::chrono::year_month_day());
}

std::chrono::year_month_day TaskModel::getEstimatedCompletion() const
{
    return estimatedCompletion.value_or(std::chrono::year_month_day());
}

std::chrono::year_month_day TaskModel::getCompletionDate() const
{
    return completionDate.value_or(std::chrono::year_month_day());
}

void TaskModel::setCreatorID(std::size_t inCreatorID)
{
    modified = true;
    creatorID = inCreatorID;
}

void TaskModel::setAssignToID(std::size_t inAssignedID)
{
    modified = true;
    assignToID = inAssignedID;
}

void TaskModel::setDescription(std::string inDescription)
{
    modified = true;
    description = inDescription;
}

void TaskModel::setStatus(TaskModel::TaskStatus inStatus)
{
    modified = true;
    status = inStatus;
}

void TaskModel::setParentTaskID(std::size_t inParentTaskID)
{
    modified = true;
    parentTaskID = inParentTaskID;
}

void TaskModel::setPercentageComplete(double inPercentComplete)
{
    modified = true;
    percentageComplete = inPercentComplete;
}

void TaskModel::setCreationDate(std::chrono::year_month_day inCreationDate)
{
    modified = true;
    creationDate = inCreationDate;
}

void TaskModel::setDueDate(std::chrono::year_month_day inDueDate)
{
    modified = true;
    dueDate = inDueDate;
}

void TaskModel::setScheduledStart(std::chrono::year_month_day startDate)
{
    modified = true;
    scheduledStart = startDate;
}

void TaskModel::setactualStartDate(std::chrono::year_month_day startDate)
{
    modified = true;
    actualStartDate = startDate;
}

void TaskModel::setEstimatedCompletion(std::chrono::year_month_day completionDate)
{
    modified = true;
    estimatedCompletion = completionDate;
}

void TaskModel::setCompletionDate(std::chrono::year_month_day inCompletionDate)
{
    modified = true;
    completionDate = inCompletionDate;
}

void TaskModel::setEstimatedEffort(unsigned int inEstimatedHours)
{
    modified = true;
    estimatedEffort = inEstimatedHours;
}

void TaskModel::setActualEffortToDate(double effortHoursYTD)
{
    modified = true;
    actualEffortToDate = effortHoursYTD;
}

void TaskModel::setPriorityGroup(unsigned int inPriorityGroup)
{
    modified = true;
    priorityGroup = inPriorityGroup;
}

void TaskModel::setPriorityGroupC(const char priorityGroup)
{
    unsigned int group = priorityGroup - 'A' + 1;
    setPriorityGroup(group);
}

void TaskModel::setPriority(unsigned int inPriority)
{
    modified = true;
    priority = inPriority;
}

void TaskModel::setPersonal(bool personalIn)
{
    modified = true;
    personal = personalIn;
}

void TaskModel::addDependency(std::size_t taskId)
{
    modified = true;
    dependencies.push_back(taskId);
}

void TaskModel::setTaskID(std::size_t newID)
{
    modified = true;
    primaryKey = newID;
}

bool TaskModel::selectByDescriptionAndAssignedUser(std::string_view description, std::size_t assignedUserID)
{
    prepareForRunQueryAsync();

    try
    {
        NSBM::format_context fctx(format_opts.value());
        NSBM::format_sql_to(fctx, baseQuery);
        NSBM::format_sql_to(fctx, " WHERE Description = {} AND AsignedTo = {}", description, assignedUserID);

        NSBM::results localResult = runQueryAsync(std::move(fctx).get().value());

        return processResult(localResult);
    }

    catch(const std::exception& e)
    {
        appendErrorMessage(std::format("In TaskModel::selectByDescriptionAndAssignedUser({}) : {}", description, e.what()));
        return false;
    }
}

bool TaskModel::selectByTaskID(std::size_t taskID)
{
    prepareForRunQueryAsync();

    try
    {
        NSBM::format_context fctx(format_opts.value());
        NSBM::format_sql_to(fctx, baseQuery);
        NSBM::format_sql_to(fctx, " WHERE TaskID = {}", taskID);

        NSBM::results localResult = runQueryAsync(std::move(fctx).get().value());

        return processResult(localResult);
    }

    catch(const std::exception& e)
    {
        appendErrorMessage(std::format("In TaskModel::selectByTaskID({}) : {}", taskID, e.what()));
        return false;
    }
}

std::string TaskModel::formatSelectActiveTasksForAssignedUser(std::size_t assignedUserID)
{
    prepareForRunQueryAsync();

    try {
        constexpr unsigned int notStarted = static_cast<unsigned int>(TaskModel::TaskStatus::Not_Started);

        NSBM::format_context fctx(format_opts.value());
        NSBM::format_sql_to(fctx, listQueryBase);
        NSBM::format_sql_to(fctx, " WHERE AsignedTo = {} AND Completed IS NULL AND (Status IS NOT NULL AND Status <> {})",
            assignedUserID, stdchronoDateToBoostMySQLDate(getTodaysDatePlus(OneWeek)), notStarted);

        return std::move(fctx).get().value();
    }

    catch(const std::exception& e)
    {
        appendErrorMessage(std::format("In TaskModel::formatSelectUnstartedDueForStartForAssignedUser({}) : {}", assignedUserID, e.what()));
    }

    return std::string();
}

std::string TaskModel::formatSelectUnstartedDueForStartForAssignedUser(std::size_t assignedUserID)
{
    prepareForRunQueryAsync();

    try {
        constexpr unsigned int notStarted = static_cast<unsigned int>(TaskModel::TaskStatus::Not_Started);

        NSBM::format_context fctx(format_opts.value());
        NSBM::format_sql_to(fctx, listQueryBase);
        NSBM::format_sql_to(fctx, " WHERE AsignedTo = {} AND ScheduledStart < {} AND (Status IS NULL OR Status = {})",
            assignedUserID, stdchronoDateToBoostMySQLDate(getTodaysDatePlus(OneWeek)), notStarted);

        return std::move(fctx).get().value();
    }

    catch(const std::exception& e)
    {
        appendErrorMessage(std::format("In TaskModel::formatSelectUnstartedDueForStartForAssignedUser({}) : {}", assignedUserID, e.what()));
    }

    return std::string();
}

std::string TaskModel::formatSelectTasksCompletedByAssignedAfterDate(std::size_t assignedUserID, std::chrono::year_month_day& searchStartDate)
{
    prepareForRunQueryAsync();

    try {
        NSBM::format_context fctx(format_opts.value());
        NSBM::format_sql_to(fctx, listQueryBase);
        NSBM::format_sql_to(fctx, " WHERE AsignedTo = {} AND Completed >= {}",
            assignedUserID, stdchronoDateToBoostMySQLDate(searchStartDate));

        return std::move(fctx).get().value();
    }

    catch(const std::exception& e)
    {
        appendErrorMessage(std::format("In TaskModel::formatSelectTasksCompletedByAssignedAfterDate({}) : {}", assignedUserID, e.what()));
    }

    return std::string();
}

std::string TaskModel::formatSelectTasksByAssignedIDandParentID(std::size_t assignedUserID, std::size_t parentID)
{
    prepareForRunQueryAsync();

    try {
        NSBM::format_context fctx(format_opts.value());
        NSBM::format_sql_to(fctx, listQueryBase);
        NSBM::format_sql_to(fctx, " WHERE AsignedTo = {} AND ParentTask = {}", assignedUserID, parentID);

        return std::move(fctx).get().value();
    }

    catch(const std::exception& e)
    {
        appendErrorMessage(std::format("In TaskModel::formatSelectTasksByAssignedIDandParentID({}) : {}", assignedUserID, e.what()));
    }

    return std::string();
}

std::string TaskModel::taskStatusString() const
{
    TaskModel::TaskStatus status = getStatus();
    auto statusName = taskStatusConversionTable.lookupName(status);
    return statusName.has_value()? *statusName : "Unknown TaskStatus Value";
}

TaskModel::TaskStatus TaskModel::stringToStatus(std::string statusName) const
{
    auto status = taskStatusConversionTable.lookupID(statusName);
    return status.has_value()? *status : UnknowStatus;
}

bool TaskModel::diffTask(TaskModel& other)
{
    // Ignoring optional fields
    return (primaryKey == other.primaryKey &&
        description == other.description &&
        other.creatorID == creatorID &&
        assignToID == other.assignToID &&
        percentageComplete == other.percentageComplete &&
        creationDate == other.creationDate &&
        dueDate == other.dueDate &&
        scheduledStart == other.scheduledStart &&
        scheduledStart == other.scheduledStart &&
        actualEffortToDate == other.actualEffortToDate &&
        priorityGroup == other.priorityGroup &&
        priority == other.priority &&
        personal == other.personal &&
        dependencies.size() == other.dependencies.size()
    );
}

std::string TaskModel::formatInsertStatement()
{
    std::size_t dependencyCount = getDependencies().size();
    std::optional<std::string> depenenciesText;
    if (dependencyCount)
    {
        std::vector<std::size_t> dependencyList = getDependencies();
        depenenciesText = buildDependenciesText(dependencyList);
    }

    return NSBM::format_sql(format_opts.value(),
        "INSERT INTO Tasks (CreatedBy, AsignedTo, Description, ParentTask, Status, PercentageComplete, CreatedOn,"
            "RequiredDelivery, ScheduledStart, ActualStart, EstimatedCompletion, Completed, EstimatedEffortHours, "
            "ActualEffortHours, SchedulePriorityGroup, PriorityInGroup, Personal, DependencyCount, Dependencies)"
            " VALUES ({0}, {1}, {2}, {3}, {4}, {5}, {6}, {7}, {8}, {9}, {10}, {11}, {12}, {13}, {14}, {15}, {16}, {17}, {18})",
            creatorID,
            assignToID,
            description,
            parentTaskID,
            getStatusIntVal(),
            percentageComplete,
            stdchronoDateToBoostMySQLDate(creationDate),
            stdchronoDateToBoostMySQLDate(dueDate),
            stdchronoDateToBoostMySQLDate(scheduledStart),
            optionalDateConversion(actualStartDate),
            optionalDateConversion(estimatedCompletion),
            optionalDateConversion(completionDate),
            estimatedEffort,
            actualEffortToDate,
            priorityGroup,
            priority,
            personal,
            dependencyCount,
            depenenciesText
    );
}

std::string TaskModel::formatUpdateStatement()
{
    std::size_t dependencyCount = getDependencies().size();
    std::optional<std::string> depenenciesText;
    if (dependencyCount)
    {
        std::vector<std::size_t> dependencyList = getDependencies();
        depenenciesText = buildDependenciesText(dependencyList);
    }

    return NSBM::format_sql(format_opts.value(),
        "UPDATE Tasks SET"
            " CreatedBy = {0},"
            " AsignedTo = {1},"
            " Description = {2},"
            " ParentTask = {3},"
            " Status = {4},"
            " PercentageComplete = {5},"
            " CreatedOn = {6},"
            " RequiredDelivery = {7},"
            " ScheduledStart = {8},"
            " ActualStart = {9},"
            " EstimatedCompletion = {10},"
            " Completed = {11},"
            " EstimatedEffortHours = {12},"
            " ActualEffortHours = {13},"
            " SchedulePriorityGroup = {14},"
            " PriorityInGroup = {15},"
            " Personal = {16},"
            " DependencyCount = {17},"
            "Dependencies = {18}"
        " WHERE TaskID = {19} ",
            creatorID,
            assignToID,
            description,
            parentTaskID,
            getStatusIntVal(),
            percentageComplete,
            stdchronoDateToBoostMySQLDate(creationDate),
            stdchronoDateToBoostMySQLDate(dueDate),
            stdchronoDateToBoostMySQLDate(scheduledStart),
            optionalDateConversion(actualStartDate),
            optionalDateConversion(estimatedCompletion),
            optionalDateConversion(completionDate),
            estimatedEffort,
            actualEffortToDate,
            priorityGroup,
            priority,
            personal,
            dependencyCount,
            depenenciesText,
        primaryKey
    );}

std::string TaskModel::formatSelectStatement()
{
    prepareForRunQueryAsync();

    NSBM::format_context fctx(format_opts.value());
    NSBM::format_sql_to(fctx, baseQuery);
    NSBM::format_sql_to(fctx, " WHERE TaskID = {}", primaryKey);

    return std::move(fctx).get().value();
}

void TaskModel::initRequiredFields()
{
    missingRequiredFieldsTests.push_back({std::bind(&TaskModel::isMissingDescription, this), "description"});
    missingRequiredFieldsTests.push_back({std::bind(&TaskModel::isMissingCreatorID, this), "user ID for creator"});
    missingRequiredFieldsTests.push_back({std::bind(&TaskModel::isMissingAssignedID, this), "user ID for assigned user"});
    missingRequiredFieldsTests.push_back({std::bind(&TaskModel::isMissingEffortEstimate, this), "estimated effort in hours"});
    missingRequiredFieldsTests.push_back({std::bind(&TaskModel::isMissingPriorityGroup, this), "priority"});
    missingRequiredFieldsTests.push_back({std::bind(&TaskModel::isMissingCreationDate, this), "date of creation"});
    missingRequiredFieldsTests.push_back({std::bind(&TaskModel::isMissingScheduledStart, this), "scheduled start date"});
    missingRequiredFieldsTests.push_back({std::bind(&TaskModel::isMissingDueDate, this), "due date (deadline)"});
}

void TaskModel::addDependencies(const std::string & dependenciesText)
{
    std::vector<std::string> dependencyStrings = explodeTextField(dependenciesText);

    if (!dependencyStrings.empty())
    {
        for (auto dependencyStr: dependencyStrings)
        {
            dependencies.push_back(static_cast<std::size_t>(std::stol(dependencyStr)));
        }
    }
    else
    {
        std::runtime_error NoExpectedDependencies("Dependencies expected but not found!");
        throw NoExpectedDependencies;
    }
}

std::string TaskModel::buildDependenciesText(std::vector<std::size_t>& dependencyList) noexcept
{
    if (dependencyList.size() > 1)
    {
        std::sort(dependencyList.begin(), dependencyList.end());
    }

    std::vector<std::string> dependencyStrings;
    for (auto dependency: dependencyList)
    {
        dependencyStrings.push_back(std::to_string(dependency));
    }

    return implodeTextField(dependencyStrings);
}

void TaskModel::processResultRow(NSBM::row_view rv)
{
    // Required fields.
    primaryKey = rv.at(taskIdIdx).as_uint64();
    creatorID = rv.at(createdByIdx).as_uint64();
    assignToID = rv.at(assignedToIdx).as_uint64();
    description = rv.at(descriptionIdx).as_string();
    percentageComplete = rv.at(percentageCompleteIdx).as_double();
    creationDate = boostMysqlDateToChronoDate(rv.at(createdOnIdx).as_date());
    dueDate = boostMysqlDateToChronoDate(rv.at(requiredDeliveryIdx).as_date());
    scheduledStart = boostMysqlDateToChronoDate(rv.at(scheduledStartIdx).as_date());
    estimatedEffort = rv.at(estimatedEffortHoursIdx).as_uint64();
    actualEffortToDate = rv.at(actualEffortHoursIdx).as_double();
    priorityGroup = rv.at(schedulePriorityGroupIdx).as_uint64();
    priority = rv.at(priorityInGroupIdx).as_uint64();
    personal = rv.at(personalIdx).as_int64();

    // Optional fields.
    if (!rv.at(parentTaskIdx).is_null())
    {
        parentTaskID = rv.at(parentTaskIdx).as_uint64();
    }

    if (!rv.at(statusIdx).is_null())
    {
        setStatus(static_cast<TaskModel::TaskStatus>(rv.at(statusIdx).as_uint64()));
    }

    if (!rv.at(actualStartIdx).is_null())
    {
        actualStartDate = boostMysqlDateToChronoDate(rv.at(actualStartIdx).as_date());
    }

    if (!rv.at(estimatedCompletionIdx).is_null())
    {
        estimatedCompletion = boostMysqlDateToChronoDate(rv.at(estimatedCompletionIdx).as_date());
    }

    if (!rv.at(completedIdx).is_null())
    {
        completionDate = boostMysqlDateToChronoDate(rv.at(completedIdx).as_date());
    }

    std::size_t dependencyCount = rv.at(dependencyCountIdx).as_uint64();
    if (dependencyCount > 0)
    {
        std::string dependenciesText = rv.at(depenedenciesTextIdx).as_string();
        addDependencies(dependenciesText);
    }

    // All the set functions set modified, since this user is new in memory it is not modified.
    modified = false;

}

TaskList.h

#ifndef TASKLIST_H_
#define TASKLIST_H_

#include <chrono>
#include <format>
#include <iostream>
#include "ListDBInterface.h"
#include "TaskModel.h"

using TaskListValues = std::vector<TaskModel_shp>;

class TaskList : public ListDBInterface<TaskModel>
{
public:
    TaskList();
    virtual ~TaskList() = default;

    TaskListValues getActiveTasksForAssignedUser(std::size_t assignedUserID);
    TaskListValues getUnstartedDueForStartForAssignedUser(std::size_t assignedUserID);
    TaskListValues getTasksCompletedByAssignedAfterDate(std::size_t assignedUserID,
        std::chrono::year_month_day& searchStartDate);
    TaskListValues getTasksByAssignedIDandParentID(std::size_t assignedUserID, std::size_t parentID);

private:
    TaskListValues fillTaskList();
    TaskListValues runQueryFillTaskList();

};

#endif // TASKLIST_H_

TaskList.cpp

#include <chrono>
#include <format>
#include <iostream>
#include "ListDBInterface.h"
#include "TaskList.h"
#include "TaskModel.h"

TaskList::TaskList()
: ListDBInterface<TaskModel>()
{

}

TaskListValues TaskList::getActiveTasksForAssignedUser(std::size_t assignedUserID)
{
    prepareForRunQueryAsync();
/*
 * Prepend function name to any error messages.
 */
    appendErrorMessage("In TaskList::getActiveTasksForAssignedUser : ");

    try
    {
        firstFormattedQuery = queryGenerator.formatSelectActiveTasksForAssignedUser(assignedUserID);
        return runQueryFillTaskList();
    }

    catch(const std::exception& e)
    {
        appendErrorMessage(e.what());
    }
    
    return TaskListValues();
}

TaskListValues TaskList::getUnstartedDueForStartForAssignedUser(std::size_t assignedUserID)
{
    prepareForRunQueryAsync();
    appendErrorMessage("In TaskList::getUnstartedDueForStartForAssignedUser : ");

    try
    {
        firstFormattedQuery = queryGenerator.formatSelectUnstartedDueForStartForAssignedUser(assignedUserID);
        return runQueryFillTaskList();
    }

    catch(const std::exception& e)
    {
        appendErrorMessage(e.what());
    }
    
    return TaskListValues();
}

TaskListValues TaskList::getTasksCompletedByAssignedAfterDate(std::size_t assignedUserID,
    std::chrono::year_month_day& searchStartDate)
{
    prepareForRunQueryAsync();
    appendErrorMessage("In TaskList::getTasksCompletedByAssignedAfterDate : ");

    try
    {
        firstFormattedQuery = queryGenerator.formatSelectTasksCompletedByAssignedAfterDate(
            assignedUserID, searchStartDate);
        return runQueryFillTaskList();
    }

    catch(const std::exception& e)
    {
        appendErrorMessage(e.what());
    }
    
    return TaskListValues();
}

TaskListValues TaskList::getTasksByAssignedIDandParentID(std::size_t assignedUserID, std::size_t parentID)
{
    prepareForRunQueryAsync();
    appendErrorMessage("In TaskList::getTasksByAssignedIDandParentID : ");

    try
    {
        firstFormattedQuery = queryGenerator.formatSelectTasksByAssignedIDandParentID(
            assignedUserID, parentID);
        return runQueryFillTaskList();
    }

    catch(const std::exception& e)
    {
        appendErrorMessage(e.what());
    }
    
    return TaskListValues();
}

TaskListValues TaskList::fillTaskList()
{
    TaskListValues taskList;

    for (auto taskID: primaryKeyResults)
    {
        TaskModel_shp newTask = std::make_shared<TaskModel>(TaskModel());
        newTask->selectByTaskID(taskID);
        taskList.push_back(newTask);
    }

    return taskList;
}

TaskListValues TaskList::runQueryFillTaskList()
{
    if (firstFormattedQuery.empty())
    {
        appendErrorMessage(std::format("Formatting select multiple tasks query string failed {}",
            queryGenerator.getAllErrorMessages()));
        return TaskListValues();
    }
    if (runFirstQuery())
    {
        return fillTaskList();
    }

    return TaskListValues();
}
```
\$\endgroup\$
1
  • 1
    \$\begingroup\$ When linking to a git repo, you can be extra helpful by linking to the specific commit that you have posted for review (even better, you might create a tag on that commit). Just a suggestion, not a requirement. \$\endgroup\$ Commented Sep 22 at 6:58

2 Answers 2

Reset to default
3
+100
\$\begingroup\$

overview

Here's my high level impression of the design behind this 1.5 KLoC codebase.

It enjoys a great deal of consistency, including naming consistency. Clearly much care went into it.

I share the OP's concerns about maintainability, and code reduction. Table schemas change all the time, columns come and go and are renamed; that's why we choose the flexibility of an RDBMS. The current code requires significantly more re-work, in more places, than I am accustomed to when performing such maintenance tasks.

storing the schema

There's pretty much two places to store column names and constraints.

  1. inside the RDBMS
  2. outside, such as in *.cpp source files, or perhaps a config file

My personal preference is that in an evolving system, where columns change, the RDBMS should be the "source of truth", so we can rely on relational integrity.

Then at run time we can either

  • A. Dynamically query the DB for schema details, and cache them in memory, or
  • B. Occasionally run a report against the schema and use that to generate config files or *.cpp source files.

A single code base might interact with a "new" test DB and with an "old" production DB that does not yet have some recently added column.

A GenericDictionary can map from a std::string columnName to a struct / tuple of (type, isNullable).

In this case the OP code has gone pretty far down the path of using option (2).

credentials

protoPersonalPlanner -u MySQLUser -p MySQLPassword ...

Secret passwords never belong checked in to a repo, even if the DB currently is not accessible via internet. Standard advice is to read them from a (.gitignore'd) config file, from a vault utility, or from env vars. That last one enjoys some popularity since it's easily adapted to Bourne, C++, and other languages.

I like your programOptions, but I guess it's not very bash friendly.

Also, I think checking in testOut_forDiff.txt is a great idea; I often do something similar. But nit: prefer diff -u, so in the event of error it's easier to read.

DbC

In initFormatOptions() it's not obvious to me that we need a try at all. It might be better to let a failure be fatal, producing a default cerr side effect.

Upon exit the post-condition appears to be "format_opts shall be filled in", yet the error "handler" does not ensure that.

It's also not obvious why prepareForRunQueryAsync() needs to perform that init before every single INSERT that we issue.

public interface docs

This is terrific, very helpful:

/*
 * All calls to runQueryAsync should be implemented within try blocks.
 */

I don't know exactly how documentation works in your environment; I didn't notice Doxygen being used. But this comment feels like it may deserve e.g. /** instead of /*, since instead of a private implementation detail it is offering crucial advice that each call site needs to be aware of.

Taking quite a different tack, maybe supply a convenience wrapper which offers some default try / catch behavior? You know, the sort of thing that caller can't possibly mess up, even if it was coded in a bit of a hurry. When designing a Public API we strive to make it easy to call, and hard to silently accidentally mess up the details. (See errno + chapter 2 write() for a classic example of a design that works pretty well, but not always perfectly, since many call sites neglect to check the details.)

plural

nit: We have a row set here, so maybe call it results, to match the type?

    NSBM::results localResult;
    ...
    ... , NSBM::results result)

elapsed query time

In coRoutineExecuteSqlStatement() the verbosity control is very nice. Consider logging start + end timestamps. Or better, report elapsed number of seconds upon returning SELECT results.

Some "reporting" queries may run for an hour, yet start producing result rows after just a brief delay. I note in passing that this particular function seems to transfer control only after all result rows become available. That's great for many consumers, but maybe worth /* mentioning */ in case we have any reporting consumers sending in expensive queries.

vague identifier

        [&localResult, this](std::exception_ptr ptr, ...)
        {
            if (ptr)
            {
                std::rethrow_exception(ptr);

Many things are pointers. Maybe call this e, or ep, or exc?

plural, again

This is a helpful comment, but sorry, I didn't quite parse it. The below emphasis on each PK term is mine.

   ... first query will
 * only return the primary KEY of each instance of the model that matches the
 * search parameters of the query. Once the list of primary KEYS is established
 * the model will be used to select the object by primary KEY for the full data.

I think my mis-parse is due to different assumptions.

  • me: a relation can have zero or more PK columns, e.g. a compound primary key comprised of two columns
  • you: each relation used by caller shall have exactly one PK column

(And for the record, I view a relation with zero PK columns as "a text file", and I'll work to eliminate such nonsense from projects I collaborate on.)

If I properly stated the assumptions, then please write some comments about "only a single-column PK shall be supported", something like that.

Hmmm, maybe this library says that only uint64 can serve as PK?

nit, typo: "that theRE is a maximum size"

nit, typo: "too much OF a performance ..."

nit, concept: "large amount of data" could mean more than a million "skinny" two-column rows with integer results, and could also mean fewer "fat" rows containing giant BLOBs or a great many columns. When we look at sizing buffers we typically don't care which one, but here it looks like they may present two different cases that the code should separately measure and otherwise worry about.

I view "return all matching PKs" as weird. Maybe we have timing results already, which justify this design decision? Typically we return queried result columns along with PK column(s).

Making it easier for caller to paginate a query with offset + LIMIT is the more usual approach for bounding memory consumption. (Plus pagination lets the caller decide the results are no longer interesting, and hence cancel before a million rows are retrieved.)

many-to-many

The current code can easily accommodate a one-to-many FK schema.

It's common to encounter a many-to-many association table, with a compound PK of two columns, each having a foreign key relationship to Table A or to Table B. The OP code does not seem to support such a schema. But if you don't need it, hey, that's great, more power to you.

more columns

It used to be said of Fortran routines that "if it accepts six or seven arguments, there's probably another two that are still missing." My first impression of some of the various tables is that maybe they're a bit wide, with lots of columns? Or maybe we're in 3NF and each column is essential. IDK. My prediction is the evolving project will add more than one column and maybe another table or two that participates in a JOIN.

uninitialized

Thank you for documenting this, and for the careful attention to detail.

... suppressed errors ... are caused by checking std::chrono::year_day_model::ok() on uninitialized .. variables in the TaskModel.

Maybe this is a minor rough edge that you'd like to fix up at this point? By ensuring the test inits each value prior to the ok()? I suppose a default date of 1st January 1970 would be fairly natural, corresponding to a unixtime of 0. And then a simple audit query could occasionally verify that nothing prior to 1971 has been persisted.

upsert

Consider renaming save() to upsert(). Or at least mention the semantics in a comment.

And yes, I see we're not using INSERT ... ON DUPLICATE KEY UPDATE, nor IGNORE nor REPLACE.

If this code has ambitions of sticking to just SQL-92 syntax, for portability across backend DB vendors, then write it down as an explicit comment.

write performance

I don't know whether your planned workload involves a ton of writes. But I can tell you that if so, with MySQL and other RDBMSs it is much faster to issue a small number of bulk insert calls to the backend, rather than a large number of single-row inserts. It makes more information available to the Query Planner so it can do a better job of scheduling the I/O. In particular, you would much prefer to COMMIT on every thousandth row than on every row inserted.

relational consistency

This seems like a helpful comment.

/*
 * Assumes that ModelDBInterface::hasRequiredValues() was called previously and that
 * initRequiredFields() has been called.
 */

But it makes me wonder just where the dotted line on the whiteboard should go, to describe the part of the system where certain things are guaranteed.

In an OOP setting, I'm accustomed to "class invariant", where it is impossible to construct an object that violates the invariant, and then all methods promise to preserve the invariant. In an RDBMS setting, I'm accustomed to the boundary being drawn at the DB driver level, with DDL enforcing constraints like NOT NULL. And we can ask the backend DB at runtime about the set of columns currently protected by a NOT NULL constraint.

So I'm not saying anything terrible about this particular bit of code; I'm just saying it's slightly odd, it makes me nervous. My usual expectations don't seem to apply, which could make maintenance tasks a bit trickier. Maybe expose a public method that calls a pair of private helpers?

summary: If you look at any RDBMS you'll find it is Boring. And boring is Good. No surprises.

SI units

    const std::size_t percentageCompleteIdx = ...
    ...
    const std::size_t estimatedEffortHoursIdx = ...

Consider preferring an "hours completed", and/or an "est. hours remaining" column, rather than a percentage. If the Agile movement has shown us one thing, it is that "80% complete" is a figment of the imagination. Either a thing is Done or it is not, and if it drags on too long it should be decomposed to sub-tasks.

LIKE

        NSBM::format_sql_to(fctx,
            " WHERE Description = {} AND AsignedTo = {}", ... );

Looks good.

For the 2nd and especially the 1st parameter, consider replacing = with LIKE. This admits of wildcards. I suspect some descriptions may be tediously long, to the point where they interfere with the UX.

Prefer trailing wildcard of "balance books%" since it plays nice with CREATE INDEX. A leading wildcard of "%balance books%" will typically disable the index and instead force a tablescan.

boolean column

Well, this one surprised me.

... AND Completed IS NULL AND ...

I had been reading that as BOOL. And almost always the CREATE TABLE should mention ... BOOL NOT NULL.

More generally, tri-valued logic is perfectly obvious to the machine, but humans tend to find it a bit of a speed bump when reasoning about it. So where feasible, put a NOT NULL clause into your DDL, and you should almost always tack NOT NULL onto any boolean columns.

I had anticipated that I would read ... AND Completed AND ...

relational integrity, redux

std::string TaskModel::taskStatusString() const
{
    TaskModel::TaskStatus status = getStatus();
    auto statusName = taskStatusConversionTable.lookupName(status);
    return statusName.has_value()? *statusName : "Unknown TaskStatus Value";
}

In a relational setting, that seems slightly odd. Typically the backend should be responsible for such validation.

Consider using ENUM in the CREATE TABLE statement, or using JOIN against a status conversion table. It's a little jarring that we could persist an "unknown" value.

relational model

I see we're storing a DependencyCount column, right next to Dependencies. Both of those seem odd.

Typically we'd have a many-to-one Dependency table with FK relationship to Tasks, and the count would be found with COUNT(*) from a JOIN.

There can be good reasons to denorm. It's a good idea to write them down, so a future maintenance engineer can revisit them and perhaps arrive at a different set of tradeoffs.

\$\endgroup\$
7
  • \$\begingroup\$ Just FYI, the original relational design of the task portion of the database. \$\endgroup\$ Commented Sep 21 at 11:57
  • \$\begingroup\$ You are correct, user name and passwords have no place in a git repository, which is why you won't find the test script in the main repository and why generic user names and passwords are provided here. If anyone wants to test the code they will have to replace the user name and password with their own. It is also why the valgrind diff will always have at least one line diff since the command has been removed from the comparison text file. \$\endgroup\$ Commented Sep 21 at 14:02
  • \$\begingroup\$ While objects coming from the database will always have required dates initialized, there is no guarantee that new objects coming from the UI will have the required values initialized. I can't think of a good date to initialize dates to for the default constructor. I'm open to suggestions. \$\endgroup\$ Commented Sep 21 at 14:08
  • \$\begingroup\$ I originally tried to put the initialization of the format_options into the constructor. g++ didn't like an asynchronous database call in the constructor, therefore the very first execution of formatting a SQL statement needs to get the format options from the database server. If the format options have been set, initFormatOptions() returns without running the database access. The format options seem to be based on the formatting used in the database. A feature of boost::mysql. \$\endgroup\$ Commented Sep 21 at 14:27
  • \$\begingroup\$ I like ERDs; I think they're a great visualization. My preference is to auto-generate them from a live database's DDL / schema, so each new revision of the diagram will "tell the truth", will correspond exactly to what's running in production. \$\endgroup\$ Commented Sep 21 at 15:06
3
\$\begingroup\$

J_H's answer has done a great job of reviewing the design. I have only implementation nitpicks.

Let's consider the includes from TaskModel.cpp:

#include <chrono>
#include "commonUtilities.h"
#include <functional>
#include "GenericDictionary.h"
#include <iostream>
#include <memory>
#include <string>
#include "TaskModel.h"
//#include "UserModel.h"
#include <vector>

I see that these are alphabetical. It's good to have a guiding principle, but I would do things slightly differently:

  • Include "TaskModel.h" as the very first header. That ensures there's at least one translation unit that ensures that this one is complete and self-contained, i.e. doesn't have a hidden dependency on some other header being included earlier.
  • Separate the project headers from the standard library headers. That's more of a personal preference - I just find it easier if the headers from one source are all together. I apply the same thing if I use any other C++ libraries - give each its own block of includes.
  • Remove any that are not needed, rather than commenting out. That's a good principle in general, especially as you are using a source-code control system (you're not in danger of losing knowledge from earlier versions).

The resultant layout would look like this in my world:

#include "TaskModel.h"

#include "commonUtilities.h"
#include "GenericDictionary.h"

#include <chrono>
#include <functional>
#include <iostream>
#include <memory>
#include <string>
#include <vector>

In passing, I note the inconsistency between Pascal-case GenericDictionary and camel-case commonUtilities. Perhaps there's a good reason for the difference in convention, but to me that's just going to be frustrating.

Naming: I don't like the use of all-caps for NSBA and NSBM, which established C and C++ practice suggests to be preprocessor macros. They are also hard to pronounce, and visually too similar to each other.

I wouldn't have declared these at global scope in a header, which brings them in everywhere that includes it. I'd probably have gone for something like

class CoreDBInterface
{
    namespace asio = boost::asio;
    namespace mysql = boost::mysql;

Implementation files can still locally alias these, of course, without affecting client code.

Some files are missing definitions that are clearly picked up by chance in your environment, but not guaranteed by the C++ standard. A handful of examples:

  • std::exception in CoreDBInterface.cpp
  • std::exception_ptr ptr and std::rethrow_exception in the same source
  • std::move, also in CoreDBInterface.cpp
  • std::exception in ListDBInterface.h
  • std::format in ModelDBInterface.cpp

Consider using a tool such as include-what-you-use to help ensure that source files are complete.

Think about which operations that users would consider const. In ModelDBInterface, I would expect retrieve() and hasRequiredValues() not to make changes to the stored data and therefore be const. Consider making fields mutable as necessary to give a consistent and intuitive model of constness that corresponds to user-observable state.

There's probably an entire review to be had of the unit testing and regression suite. Understandably, you haven't included the testing code (other than the driver script) in this already large review request. But from what I can see, it always exits with a success status unless the valgrind output couldn't be generated, which makes it unsuitable as a build step.

I'm assuming the unit tests each take place in a transaction that's aborted at the end of each test? That's what I did last time I did SQL development (albeit I was working on stored procedures rather than client code). If possible, consider a local DB such as SQLite to allow you to run fast in-memory tests without the overhead (and potential source of runtime error) involved in connecting to a database server.

As I said, the testing is worth a review of its own; I won't go any further here.

\$\endgroup\$
5
  • 1
    \$\begingroup\$ The reason for the file naming irregularity, all headers that contain classes are named by the classes and each class is Capitalized, any headers that contain only functions are camelCase. GenericDictionary is a template class, commonUtilities is a bunch of functions. \$\endgroup\$ Commented Sep 22 at 11:38
  • \$\begingroup\$ About repository branching. I generally do this and the development directory does have several branches. The repository that I pointed to for the question was created just for the question to make things easier for others viewing the code. It is completely separate from the development repository, but you are correct I need to create a branch in the development repository for the code review. \$\endgroup\$ Commented Sep 22 at 11:41
  • 1
    \$\begingroup\$ The branch in the main development repository that contains the code review is preTaskCodeReview. \$\endgroup\$ Commented Sep 22 at 12:51
  • \$\begingroup\$ My first issue! github.com/pacmaninbw/CCTaskManagementAndScheduler/issues/1 \$\endgroup\$ Commented Sep 22 at 13:55
  • 2
    \$\begingroup\$ Yay, issues! I use them to generate a unique numeric identifier N. (If the Title is self explanatory then I might not write anything more in the body.) Suppose the title is "missing headers". With a newly assigned N in hand, I will immediately create a new feature branch named N-missing-headers, e.g. 1-missing-headers. And then git pull, switch to the branch, and start hacking. The branch name is a reminder about "what edits belong here", which helps me avoid adding everything & the kitchen sink to it. Collaborators can see what I'm working on. And by next sprint we should merge / delete it. \$\endgroup\$ Commented Sep 22 at 17:46

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.