1
\$\begingroup\$

To improve my knowledge I am trying to make a bool class from scratch, or derivative of. I am using MinGW g++11 compiler on a Windows 7 laptop.

#include <iostream>
#include <bitset>

class Zeroth{

    uint8_t* MEM = nullptr;

    public:
        const static uint8_t NullSet; // if data is declared sttic it is shared acros all instances, const means it can not be changed
        const static uint8_t UnitSet;

        /*
            the idea is that instead copying integers i can define a constant shared memories [ generated on a fly or in a header ]
            and then just store memory of the value that is pointed to 
            so if object is present: point to memory of UnitSet
            if object isn't present: point to memory of NullSet

            therefore i have 1 8 bit space instanced and deleted with every value, but all those spaces point to the same constant memories
        */

        Zeroth():MEM(new uint8_t){ this->MEM = (uint8_t*)&Zeroth::NullSet; }
        Zeroth(int VAL):MEM(new uint8_t) {
            this->MEM = (uint8_t*)( VAL&0x1 ? &Zeroth::UnitSet : &Zeroth::NullSet );
        }
        ~Zeroth(){ delete MEM; }

        const bool isPresent() const {
            return (int)&Zeroth::UnitSet == (int)&(*this->MEM);
        }
        const bool isNotPresent() const {
            return (int)&Zeroth::NullSet == (int)&(*this->MEM);
        }

        friend std::ostream& operator<<( std::ostream& os, const Zeroth& data );
};
std::ostream& operator<<( std::ostream& os, const Zeroth &data ){
    return os << ( data.isPresent() ? 'T' : 'F');
}

const uint8_t Zeroth::NullSet = 0;
const uint8_t Zeroth::UnitSet = 1;

int main(){

    Zeroth Az(3);
    std::cout << Az << std::endl;
    std::cout << Az.isPresent() << std::endl;
    std::cout << Az.isNotPresent() << std::endl;

    Zeroth Bz;
    std::cout << Bz << std::endl;
    std::cout << Bz.isPresent() << std::endl;
    std::cout << Bz.isNotPresent() << std::endl;



    return 0;
}

And result is :

CD: F:\cppDMmods
Current directory: F:\cppDMmods
g++  -Wno-unused-variable -O2 F:\cppDMmods\test.cpp -o F:\cppDMmods\test.exe
Process started (PID=4296) >>>
<<< Process finished (PID=4296). (Exit code 0)

IF: "Exists" != "Exists" goto NOEXE
test.exe
Process started (PID=5516) >>>

T // Az printout
1 // Az "presence" :> aka true
0 // Az "negated presence" :> aka !true
F // Bz printout
0 // Bz "presence"
1 // Bz "negated presence"

<<< Process finished (PID=5516). (Exit code 0)
cmd /c del test.exe
Process started (PID=6124) >>>
<<< Process finished (PID=6124). (Exit code 0)
NO EXE FILE TO DELETE
================ READY ================

I am worried that due to my presumed lack of knowledge there might be some memory leaks, massive future bugs and etc.
Any suggestions, recommendations and pointed out issues are welcomed.

\$\endgroup\$
  • 1
    \$\begingroup\$ The size of a pointer to an uint8_t should be either 32bit or 64bit and is therefore heavier than if you'd use the value itself. \$\endgroup\$ – AlexV Jul 14 at 21:32
  • \$\begingroup\$ Are you talking about MEM member ? How can i declare an 34 bit ( by defining an uint64_t ? ) , and what are possible issues that could rise if i don't change the size of the pointer ? \$\endgroup\$ – Danilo Jul 14 at 21:35
  • 1
    \$\begingroup\$ The pointer has the right size, C++ will take care of this. What I was trying to say is that "instead copying integers i [...] just store memory [address] of the value" (from your code) increases the memory footprint (see yourself in this online example: cpp.sh/2xmm). \$\endgroup\$ – AlexV Jul 14 at 21:49
  • \$\begingroup\$ So , i am guessing the same thing arises if i try to store address of instead of pointer? Which makes sense since size of an address is tied to OS bit size... and since we push the elements on the registry the variable is destroyed once it exits the scope. So bool a = true expands to bool a = new bool(1) ? \$\endgroup\$ – Danilo Jul 14 at 22:03
1
\$\begingroup\$

I'm sorry to be so straightforward: This is madness! Your code seems to indicate a sincere lack of several language features.

Pointer to const

You define MEM to be of type uint8_t*, but it actually points to values of const uint8_t. To account for this difference you cast the const away in both constructors. If you'd ever (accidentally) write to the memory MEM points to, undefined behavior might ensue. So it should be at least of type uint8_t const *.

Memory leak in constructor

As you have suspected, your code leaks memory. Both constructors assign new uint8_t to MEM before overwriting it directly afterwards. Now there is no way to reference the values created on the heap with new. Both allocations are not necessary since MEM always points to one of the two static variables. Therefore you should also not try to delete them once Zeroth goes out of scope.

Since we are at the constructor: why do you accept an 32bit integer as argument when all you care about is one bit and all your internal values are stored in an uint8_t? I also find it unintuitive to treat all the even numbers as false, and all the odd ones as true (this is basically a consequence of your bit mask). Often all values except 0 are treated as truthy values (exempt of this rule are exit codes, where 0 is usually seen as success).

isPresent/isNotPresent

Those functions do not even compile on a 64bit platform, since the completely unnecessary cast to int narrows the bidwidth of the pointer type from 64bit to 32bit. Apart from that, there is a whole lot of other weird things going on here.

Let's try to break this down:

(int)&(*this->MEM)

this->MEM is a pointer. You then for whatever reason decided to dereference (*this->MEM) it to get the value it's pointing to. After that, you take the address of the value (&(*this->MEM)), which should be the same as this->MEM. And if all of this weren't enough, you then try to cram the address into an int ((int)&(*this->MEM)).

Same procedure for the value you try to compare it with:

(int)&Zeroth::UnitSet

You have the value Zeroth::UnitSet, take the address of it &Zeroth::UnitSet and then try to cram this address into a 32bit integer.

You get the same effect using

bool isPresent() const { return &Zeroth::UnitSet == this->MEM; }
bool isNotPresent() const { return &Zeroth::NullSet == this->MEM; }

without all this madness.

Memory footprint

The internal state of your class is represented by a pointer to a const uint8_t. As I have already tried to tell you in the comments, the size of the pointer is 32bit or 64bit depending on which type of architecture your compiler tries to target. This 4 to 8 times larger than the 8bit you would need to store the value. If you stick with the value, there are far less possibilities to shoot yourself in the foot.

class ZerothVal {

public:
    ZerothVal() = default;  // see https://en.cppreference.com/w/cpp/language/default_constructor

    ZerothVal(uint8_t VAL) {
        val = (VAL & 0x1 ? ZerothVal::UnitSet : ZerothVal::NullSet);
    }

    bool isPresent() const { return val == ZerothVal::UnitSet; }
    bool isNotPresent() const { return val == ZerothVal::NullSet; }

    friend std::ostream& operator<<(std::ostream& os, const ZerothVal& data);

private:
    const uint8_t NullSet = 0;
    const uint8_t UnitSet = 1;

    uint8_t val = ZerothVal::NullSet;
};

std::ostream& operator<<(std::ostream& os, const ZerothVal& data) {
    return os << (data.isPresent() ? 'T' : 'F');
}

Operator support

Since you'd like to implement some kind of bool value, I'd highly recommend implementing the Safe Bool Idiom. For you that would mean that you have to implement operator bool() for your class (see this SO post for an explanation).

\$\endgroup\$
  • \$\begingroup\$ This is madness i tell you! Madness!! :D Don't apologise for straightforwardness and bluntness ...that is why i am here for. Operator overloading will come after i stop shooting myself in the foot. It does compile on 64 bit OS ( i am currently on it ), but this could be IDE /compiler difference. Why did you remove static ? Ok i get that all that memory , but the idea is to use shared members so each class don't have to initialise them ? Does const in this manner do the similar action ? \$\endgroup\$ – Danilo Jul 15 at 11:53
  • \$\begingroup\$ There are 32bit and 64bit versions of Cygwin, so it depends on which version you've got. I removed static because IMHO it does not buy you anything here. Until you have serious indications that there will be a problem in your application, you should stick with the simplest possible implementation. Then if there is a problem, measure carefully and then and only then look at the results and think about optimization. \$\endgroup\$ – AlexV Jul 15 at 12:03

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.