How do I properly initialize these variables?

Question

UPDATE 2:

It is compiling now but still giving the same warnings. I am stumped.

UPDATE:

Using these constructors had no effect, and still produces the same warnings. From what I understand, this is how variables are initialized. My guess is I am making a simple mistake as a beginner.

Ship::Ship()
{
    m_type[0] = '\0';
    m_engCnt = 0;
    m_engines[0] = {};
}

Engine::Engine()
{
    m_size = 0;
    m_type[0] = '\0';
}

WARNINGS:

Variable 'sdds::Ship::m_engines' is uninitialized.  Always initialize a member variable (type.6)

Variable 'sdds::EngineShip::m_type' is uninitialized.  Always initialize a member variable (type.6)

Variable 'sdds::Engine::m_size' is uninitialized.  Always initialize a member variable (type.6)

Using uninitialized memory 'invalid' -> MAIN.CPP  Line 50
Using uninitialized memory 'invalid' -> MAIN.CPP  Line 77

Buffer overrun while writing to 'this->m_engines':  the writable size is '400' bytes, but '480' bytes might be written.  -> SHIP.CPP Line 91

When I try to initialize the variables the way I see in the examples I have found, the compiler still generates warnings of uninitialized member variables on both Engine and Ship class. I know this is causing further errors down the road.

Here is what I've tried:

// Ship.cpp

Ship::Ship() {   m_type = '\0';   m_engCnt = 0;   m_engines = {}; }

// Engine.cpp

Engine::Engine() {   m_size = 0.0;   m_type = '\0'; }

I still get the uninitialized variable warnings, and I know I am allocating memory improperly, since my overall program is failing some of the member variable validations. It also keeps looping the same error message at the end when it does run.

Since I am still new to C++, I am having difficulty finding what I am doing wrong, and I'm sure that what I'm getting wrong is something blatantly obvious and stupid. So I'd appreciate if you could point out the nose on my face so I can get unstuck!

My question is:

1. How do I properly initialize these variables?
2. Is there something obvious that I am getting wrong about the memory addresses/allocation?

Here is the offending code:

// Engine.h

#pragma once
#ifndef SDDS_ENGINE_H
#define SDDS_ENGINE_H

namespace sdds
{
    const int   TYPE_MAX_SIZE = 30;                 // Max length of the type attribute in Engine class.
    
    class Engine
    {
        private:
            double  m_size;                         // The size of an engine, as a floating point number in double precision.
            char    m_type[TYPE_MAX_SIZE + 1];      // The engine model type, as an array of chars of size TYPE_MAX_SIZE.

        public:
            Engine() = default;                     // Default constructor.
            ~Engine() = default;                    // Default destructor.
            Engine(const char* type, double size);  // Custom constructor that rx's as params: engine type, size.
            double get() const;                     // Query that returns the size of the engine.
            void display() const;                   // Query that prints to the screen the content of an object in the format [SIZE] - liters - [TYPE] <ENDL>
    };
}

#endif



---

// Engine.cpp

#define _CRT_SECURE_NO_WARNINGS
#include <iostream>
#include <cstring>
#include <iomanip>
#include "Engine.h"

using namespace std;

namespace sdds
{
    Engine::Engine(const char* type, double size)
    {
        // Validate explicit params:
        if (size > 0 && type[0] != '\0')
        {
            // Assign params to engine:
            strcpy(m_type, type);
            m_size = size;
        }
    }
    
    double Engine::get() const
    {
        return m_size;          // Return size of engine m_size.
    }

    void Engine::display() const
    {
        // Detect if Engine members are valid:
        if (m_size > 0 && m_type[0] != '\0')
        {
            // If valid, display m_size at precision 2, m_type:
            cout << fixed << setprecision(2) << m_size << " liters - " << m_type << endl;
        }
    }
}


---

// Ship.h

#pragma once
#ifndef SDDS_SHIP_H
#define SDDS_SHIP_H
#include "Engine.h"

namespace sdds
{
    const double    MIN_STD_POWER = 90.111;     // The minimum power of a ship, acc'g to the regulation.
    const double    MAX_STD_POWER = 99.999;     // The maximum power of a ship acc'g to the regulation.
    const int       MAX_NUM_ENGINES = 10;       // The maximum number of engines a ship can have.

    class Ship
    {
        Engine  m_engines[MAX_NUM_ENGINES];     // Statically allocated array of engines, of size MAX_NUM_ENGINES.
        char    m_type[TYPE_MAX_SIZE + 1];      // Ship model type, statically allocated arry of charss of TYPE_MAX_SIZE.
        int     m_engCnt;                       // The number of engines that are actually installed on the ship.

    public:
        Ship() = default;
        // ~Ship() = default;
        Ship(const char* type, const Engine arr[], int size);
        operator bool() const;
        bool operator<(double power) const;
        double calculatePower() const;
        void display() const;
        Ship& operator+=(Engine engine);
        
    };
    bool operator<(double power, const Ship& theShip);
}

#endif


---

// Ship.cpp

#define _CRT_SECURE_NO_WARNINGS
#include <iostream>
#include <iomanip>
#include <cstring>
#include "Ship.h"

using namespace std;

namespace sdds
{
    //Ship::Ship()
    //{

    //}

    Ship::Ship(const char* type, const Engine engines[], int cnt)
    {
        // Validate params:
        if (type != nullptr && engines != nullptr && cnt > 0)
        {
            // If valid, store params in current instance:
            strcpy(m_type, type);           // Copy string from params to current instance of m_type.
            m_engCnt = cnt;                 // Current instance of m_engCnt is set to param cnt.
            
            // Iterate through current instance's m_engines[i] and assign engines[i].
            for (int i = 0; i < cnt; i++)
            {
                m_engines[i] = engines[i];
            }
        }
        else 
        {
            // If params are invalid, set to a default empty state.
            m_type[0] = '\0';               // Default state for m_type = '\0'.
            m_engCnt = 0;                   // Default state for m_engCnt = 0.
        }
    }

    double Ship::calculatePower() const
    {
        double total_power = 0;
        for (int i = 0; i < m_engCnt; i++)
        {
            total_power += m_engines[i].get() * 5;
        }
        return total_power;
    }

    void Ship::display() const
    {
        if (*this)
        {
            cout << "No available data" << endl;
        }
        else
        {
            cout << m_type << "-";
            cout.setf(ios::fixed);
            cout.precision(2);
            cout.width(6);
            cout << calculatePower() << endl;
            cout.unsetf(ios::fixed);
            cout.precision(6);
            for (int i = 0; i < m_engCnt; i++)
            {
                m_engines[i].display();
            }
        }
    }

    Ship::operator bool() const
    {
        // Explain in the reflection what happens if the keyword explicit is removed, and why is it necessary.
        bool valid = true;
        m_type[0] == '\0' && m_engCnt == 0 ? valid = false : valid = true;
        return valid;
    }

    Ship& Ship::operator+=(Engine engine)
    {
        // Make sure the number of engines is less than max allowed:
        if (m_engCnt < MAX_NUM_ENGINES)
        {
            if (m_type[0] == '\0')
            {
                cout << "The object is not valid! Engine cannot be added!" << endl;     // Output error message.
            }
        }
        else
        {
            m_engines[m_engCnt + 1] = engine;
        }
        return *this;
    }

    bool Ship::operator<(double power) const
    {
        bool result = false;
        calculatePower() < power ? result = true : result = false;
        return result;
    }

    bool operator<(double power, const Ship& ship)
    {
        bool result = false;
        ship.calculatePower() > power ? result = false : result = true;
        return result;

    }

}


---

// Main.cpp

#include <iostream>
#include "Ship.h"
#include "Ship.h"
#include "Engine.h"
#include "Engine.h"

using namespace std;
using namespace sdds;

void printHeader(const char* title)
{
    char oldFill = cout.fill('-');
    cout.width(40);
    cout << "" << endl;

    cout << "|> " << title << endl;

    cout.fill('-');
    cout.width(40);
    cout << "" << endl;
    cout.fill(oldFill);
}

int main()
{
    {
        printHeader("T1: Testing Constants");

        cout << "TYPE_MAX_SIZE: " << sdds::TYPE_MAX_SIZE << endl;
        cout << "MIN_STD_POWER: " << sdds::MIN_STD_POWER << endl;
        cout << "MAX_STD_POWER: " << sdds::MAX_STD_POWER << endl;
        cout << endl;
    }

    {
        printHeader("T2: Testing Default Constructor");

        Ship invalid;
        invalid.display();
        invalid += Engine("D2", 2.1);
        cout << endl;
    }

    Engine engines[] = {
        Engine("V8", 4.4),
        Engine("V8", 5.0),
        Engine("Inline", 4.1),
        Engine("D3", 7.0),
        Engine("D0", 2.0),
        Engine("D1", 3.2),
    };

    {
        printHeader("T3: Testing Custom Constructor");
        
        Ship titanic("cruiser", engines, 6);
        titanic.display();
        cout << endl;
    }

    {
        printHeader("T4: Testing Conversion to Bool Operator");
        Ship invalid;
        Ship titanic("liner", engines, 1);

        if (invalid)
            cout << "1. Test Failed! Object should be invalid.\n";
        else
            cout << "1. Test succeeded!\n";

        if (titanic)
            cout << "2. Test succeeded!\n";
        else
            cout << "3. Test Failed! Object should be valid.\n";
        
        cout << endl;
    }

    {
        printHeader("T5: Testing += and < Operators");

        Ship titanic("liner", engines, 3);

        char type[]{ "D0" };
        while (titanic < sdds::MIN_STD_POWER)
        {
            type[1]++;
            cout << "Ship not up to standard. Required power: "
                 << sdds::MIN_STD_POWER << endl;
            titanic += Engine(type, 2.1);
        }

        titanic.display();

        if (sdds::MAX_STD_POWER < titanic)
            cout << "Too much power." << endl;
        else
            cout << "Ship doesn't exceed power regulation of: "
                 << sdds::MAX_STD_POWER << endl;
    }

    return 0;
}

Answer 1

Check your control flow.

    Engine::Engine(const char* type, double size)
    {
        // Validate explicit params:
        if (size > 0 && type[0] != '\0')
        {
            // Assign params to engine:
            strcpy(m_type, type);
            m_size = size;
        }
    }

What happens for the else conditions? m_type nad m_size are not initialized. Why can't you just set m_type = '/0' and m_size = 0?

        Engine::Engine(const char* type, double size)
    {
        // Validate explicit params:
        if (size > 0 && type[0] != '\0')
        {
            // Assign params to engine:
            strcpy(m_type, type);
            m_size = size;
        }
        else
        {
            m_type = '/0';
            m_size = size;
        }
    }

The same applies for all of your other warnings. You need to make sure all construction happens for each path.

So when looking at the Ship class check the ctor for control flow as well.

Also, you set m_engCnt only in the ctor for Ship. The problem is that you allow adding engines (which should increase this count as well). You need to account for that.

Ship& Ship::operator+=(Engine engine)
{
    // Make sure the number of engines is less than max allowed:
    if (m_engCnt < MAX_NUM_ENGINES)
    {
        if (m_type[0] == '\0')
        {
            cout << "The object is not valid! Engine cannot be added!" << endl;     // Output error message.
        }
    }
    else
    {
        m_engines[m_engCnt + 1] = engine;
        m_engCnt++;
    }
    return *this;
}

Answer 2

You aren't initializing the member variables. You are assigning to them after they are automatically initialized with garbage.

There are two ways to initialize class member variables:

1. C++11 Default member initialization
2. Member Initializer lists

I prefer both as this is the intent of the default member initialization. Use default member initialization so defaulted constructors work as expected and have sane values and user-defined constructors just need to override the variables they are directly taking in as arguments.

//Note: Both of these are redundant. Pick one.
#pragma once
#ifndef SDDS_ENGINE_H
#define SDDS_ENGINE_H

namespace sdds
{
    const int   TYPE_MAX_SIZE = 30;                 // Max length of the type attribute in Engine class.
    
    class Engine
    {
        private:
            double  m_size{0.0};                         // The size of an engine, as a floating point number in double precision.
            std::string    m_type{};      // The engine model type, as an array of chars of size TYPE_MAX_SIZE.

        public:
            Engine() = default;                     // Default constructor.
            ~Engine() = default;                    // Default destructor.
            Engine(const char* type, double size);  // Custom constructor that rx's as params: engine type, size.
            double get() const;                     // Query that returns the size of the engine.
            void display() const;                   // Query that prints to the screen the content of an object in the format [SIZE] - liters - [TYPE] <ENDL>
    };
}

#endif

    Engine::Engine(const char* type, double size)
    : m_size(size)
    , m_type(type ? type : "") //Don't pass a nullptr to std::string's C-string constructor.
    {}

Answer 3

The problem is the following line:

 Engine  m_engines[MAX_NUM_ENGINES];

But you never actually allocated memory for the engine array. So the problem is that remember when you declare an int array, you need to do something like:

int* foo = new int[size of array];

I would avoid static array in this case, because you do not know how many ships are going to be added.

There is still another problem: Again you intialized your m_engines as an array that will store Engine objects but:

m_engines[0] = {};

You are setting the first element of an object type of Engine with a string???? That looks very suspicious to me....