Igneous01
Igneous01

Reputation: 739

c++ Object is creating two instances of the same array, but in different scopes?

Im having an issue with one of my classes. The class has only 1 array<> member in it. I am building a static object of this class, and initializing the values in a function. The problem is the values are never inserted.

When I step into the debugger and look at some basic insert statements into this array, the array remains empty. However if I step into the insert function itself, I can see a 'second' array of the exact same name, storing the values as expected.

It looks to me as though there is the static outer scoped array, which has nothing in it, and a second internal version (the exact same array) that has the contents stored properly.

Is there something I am missing here? I really dont know why this is happening.

Here is the minimum source code, as per request

circularbuffer.hpp

#ifndef __ma__circularbuffer_guard
#define __ma__circularbuffer_guard


#include <array>

template < typename T, int SIZE> 
class CircularBuffer
{
private:
int _index;
int _size;
std::array<T, SIZE> _buffer;
public:
CircularBuffer() { _index = 0; _size = SIZE; }

int             length  ();
typename T&     at      (int);
void            insert  (T);
int             index   ();

private:
int     realign (int&);


};

template < typename T, int SIZE> 
int CircularBuffer<T, SIZE>::realign (int& index)
{
if (index >= _size)
{
    index -= _size;
    realign(index);
} else if (index < 0)
{
    index += _size;
    realign(index);
}
return index;
}


template < typename T, int SIZE> 
int CircularBuffer<T, SIZE>::length ()
{
return _size;
}


template < typename T, int SIZE> 
typename T& CircularBuffer<T, SIZE>::at (int index)
{
realign(index);
return _buffer.at(index);
}


template <typename T, int SIZE>
void CircularBuffer<T, SIZE>::insert (T data)
{
realign(_index);
_buffer.at(_index) = data;
_index += 1;
}


template <typename T, int SIZE>
int CircularBuffer<T, SIZE>::index ()
{
return _index;
}

#endif

global buffer initializer

#ifndef __guard__namespace__notes__
#define __guard__namespace__notes__

#include "circularbuffer.hpp"
#include <memory>

typedef CircularBuffer<char, 7> CB_Natural_T;
typedef CircularBuffer<int, 12> CB_Chromatic_T;

static CB_Natural_T WHITENOTES = CB_Natural_T();        // buffer of letter notes
static CB_Chromatic_T POSITIONS = CB_Chromatic_T();     // buffer of absolute positions on keyboard

struct Initialize
{
    Initialize()
    {
        WHITENOTES.insert('C');
        WHITENOTES.insert('D');
        WHITENOTES.insert('E');
        WHITENOTES.insert('F');
        WHITENOTES.insert('G');
        WHITENOTES.insert('A');
        WHITENOTES.insert('B');

        // Initialize all positions
        for (int i = 0; i < 12; ++i)
            POSITIONS.insert(i);
    }
};

static Initialize dummy_init_var = Initialize();

#endif

to initialize the static buffers so I can unit test my other classes.

Note class header and cpp

#ifndef __guard__note__
#define __guard__note__


#include "macros.h"
#include <string>
#include <memory>


class Note
{
public:
enum Qualities { UNKNOWN = -3, DFLAT, FLAT, NATURAL, SHARP, DSHARP };   // qualities of note
typedef DEF_PTR(Note);                                                  // pointer type
private:
char _letter [1];       // the letter of the note
std::string _name;      // the full name of the note    
int _value;             // absolute value
int _position;          // relative position
Qualities _quality;     // sharp/natural/flat quality

public:
Note();
Note(char);             // letter
Note(char, Qualities);  // letter, and quality

// setters
void sharp();       // Sets the quality of the note to 1
void Dsharp();      // Sets the quality of the note to 2
void flat();        // Sets the quality of the note to -1
void Dflat();       // Sets the quality of the note to -2
void natural();     // Sets the quality of the note to 0

// getters
char letter() const;        /* returns character letter */
std::string name() const;   /* returns true name of note */
int position() const;       /* returns relative position on keyboard */
int quality() const;        /* returns the quality of the note */
void respell() const;       /* respells a note to the nearest other note */

static pointer_type make(char);             // returns a shared pointer of a new Note
static pointer_type make(char, Qualities);  // returns a shared pointer of a new Note

// operators
bool operator ==(Note& r) const;    // Returns true if Notes are truly equal
bool operator !=(Note& r) const;    // Returns true if Notes are truly not equal

bool isEnharmonic(Note& r) const;   // Returns true if Notes are enharmonically equal
bool isNatural() const;             // Returns true if Note is natural
bool isSharp() const;               // Returns true if Note is sharp
bool isDSharp() const;              // Returns true if Note is double sharp
bool isFlat() const;                // Returns true if Note is flat
bool isDFlat() const;               // Returns true if Note is double flat

private:
void makeName();    /* sets name of Note */
};


#endif


#include "note.h"

Note::Note() 
{
_letter[1] = 'u';
_name = ""; 
_value = -1; 
_quality = UNKNOWN;
_position = -1;
}

Note::Note(char l)
{
_letter[1] = l;

// determine absolute value based on letter
switch (l)
{
case 'C':
    _value = 0; break;
case 'D':
    _value = 2; break;
case 'E':
    _value = 4; break;
case 'F':
    _value = 5; break;
case 'G':
    _value = 7; break;
case 'A':
    _value = 9; break;
case 'B':
    _value = 11; break;
default:
    _value = -1; break;
}

_quality = NATURAL;
_position = _value + _quality;
makeName();
}

Note::Note(char l, Note::Qualities q)
{
_letter[1] = l;

// determine absolute value based on letter given
switch (l)
{
case 'C':
    _value = 0; break;
case 'D':
    _value = 2; break;
case 'E':
    _value = 4; break;
case 'F':
    _value = 5; break;
case 'G':
    _value = 7; break;
case 'A':
    _value = 9; break;
case 'B':
    _value = 11; break;
default:
    _value = -1; break;
}

_quality = q;       // assert for good data
_position = _value + _quality;
makeName();
}

void Note::sharp()      { _quality = SHARP;     _position = _value + 1; makeName();}
void Note::Dsharp()     { _quality = DSHARP;    _position = _value + 2; makeName();}
void Note::flat()       { _quality = FLAT;      _position = _value - 1; makeName();}
void Note::Dflat()      { _quality = DFLAT;     _position = _value - 2; makeName();}
void Note::natural()    { _quality = NATURAL;   _position = _value;     makeName(); }

char Note::letter() const       { return _letter[1]; }
std::string Note::name() const  { return _name; }
int Note::position() const      { return _position; }
int Note::quality () const      { return _quality; }


Note::pointer_type Note::make(char l)                    { return pointer_type(new Note(l)); }
Note::pointer_type Note::make(char l, Note::Qualities q) { return pointer_type(new Note(l, q)); }

void Note::makeName()
{
_name = "";
_name += _letter[1];    // add letter to name

// find out quality, add quality to name
switch (_quality)
{
case DFLAT:
    _name += "bb"; break;
case FLAT:
    _name += "b"; break;
case SHARP:
    _name += "#"; break;
case DSHARP:
    _name += "x"; break;
case NATURAL:
    break;
default:
    _name += "u"; break;
}
}

bool Note::operator ==(Note& r) const
{
// true if letter, value, position, and quality are all equal
return (_letter[1] == r._letter[1]) && (_value == r._value) && (_position == r._position) && (_quality == r._quality);
}

bool Note::operator !=(Note& r) const
{
return !(*this == r);
}

bool Note::isEnharmonic (Note& r) const
{
return (_position == r._position);
}

bool Note::isNatural() const
{
return _quality == NATURAL;
}

bool Note::isSharp() const
{
return _quality == SHARP;
}

bool Note::isDSharp() const
{
return _quality == DSHARP;
}

bool Note::isFlat() const
{
return _quality == FLAT;
}

bool Note::isDFlat() const
{
return _quality == DFLAT;
}

I would post interval as well, but that one is very big. But basically There is this code inside one of Intervals functions called findInterval

Interval::findInterval

void Interval::findInterval(Note& bottom, Note& top)
{
int index = 0;      // temp placeholder for start position

// find where the bottom note is in relation to buffer
for (int i = 0; i < WHITENOTES.length(); ++i)
{
    if (bottom.letter() == WHITENOTES.at(i))
    {
        index = i;  // set start position to this position
        break;
    }
}

// find the interpreted interval
// starting from index, with offset of length + index
for (int i = index; i < (index + WHITENOTES.length()); ++i)
{
    if (top.letter() == WHITENOTES.at(i))
    {
        _interval = i - index;  // set interval
        break;
    }
}

// modify index to serve as the position of the bottom note
index = bottom.position();

// find the physical distance
for (int i = index; i < (index + POSITIONS.length()); ++i)
{
    if (top.position() == POSITIONS.at(i))      // values match
    {
        _distance = i - index;                  // set physical distance
        break;
    }
    else if (top.position() > 11 && ((top.position() - 11) == POSITIONS.at(i)))     // if top position is higher than octave
    {
        _distance = (i - index) + 11;
        break;
    }
}


}

It fails to set the data members here, because WHITENOTES is empty, even though i called to initialize it with a static struct.

One other thing to note, if I compile my ut_interval, the tests all come back perfect with no failures, and when i check the values of the buffers in the debugger, they show up as being \0. however it still goes through the if statements and matches the char with the letter (is this some sort of encryption on chars in the debugger?)

However, exact same #includes in ut_chord, and it fails to evaluate the intervals

Here is a sample of the interval ut, and chord ut

ut_interval

#include "../common/namespace_notes.h"
#include "../common/note.h"
#include "../common/interval.h"

#define BOOST_TEST_MODULE IntervalTest
#include <boost/test/auto_unit_test.hpp>



#define TEST_IVL(i, dist, itv, q, n) \
BOOST_CHECK(i.distance() == dist); \
BOOST_CHECK(i.interval() == i.itv); \
BOOST_CHECK(i.quality() == i.q); \
BOOST_CHECK(i.name() == n)



BOOST_AUTO_TEST_CASE(INTERVAL_UNISONS)
{
// make some notes
Note C = Note('C');
Note Cs = Note('C', Cs.SHARP);
Note Cds = Note('C', Cds.DSHARP);
Note Cf = Note('C', Cf.FLAT);
Note Cdf = Note('C', Cdf.DFLAT);

// make some intervals
Interval PUnison = Interval(C, C);
Interval AugUnison = Interval(C, Cs);
Interval Aug2Unison = Interval(C, Cds);
Interval DimUnison = Interval(C, Cf);
Interval Dim2Unison = Interval(C, Cdf);

// make sure members are accurate
TEST_IVL(PUnison, 0, UNISON, PER, "Perfect Unison");
BOOST_CHECK(PUnison.isPerfect());

TEST_IVL(AugUnison, 1, UNISON, AUG, "Augmented Unison");
BOOST_CHECK(AugUnison.isAugmented());

TEST_IVL(Aug2Unison, 2, UNISON, AUG, "Augmented Unison");
BOOST_CHECK(AugUnison.isAugmented());

TEST_IVL(DimUnison, 1, UNISON, AUG, "Augmented Unison");
BOOST_CHECK(DimUnison.isAugmented());

TEST_IVL(Dim2Unison, 2, UNISON, AUG, "Augmented Unison");
BOOST_CHECK(Dim2Unison.isAugmented());
}

ut_chord

#include "../common/namespace_notes.h"
#include "../common/note.h"
#include "../common/interval.h"
#include "../common/chord.h"

#define BOOST_TEST_MODULE ChordTest
#include <boost/test/auto_unit_test.hpp>
#include <memory>


BOOST_AUTO_TEST_CASE(ChordConstructor)
{
typedef std::shared_ptr<Note> nt;
nt C = nt(new Note('C'));
nt E = nt(new Note('E'));
nt G = nt(new Note('G'));
nt B = nt(new Note('B'));

Interval PUnison = Interval(*C, *C); // cannot determine this interval
Chord C7 = Chord(C , E, G, B);
Chord C72 = Chord(B, G, E, C);
Chord C73 = Chord(E, G, C, B);
}

Upvotes: 2

Views: 655

Answers (2)

Ulrich Eckhardt
Ulrich Eckhardt

Reputation: 17415

static CB_Natural_T WHITENOTES = CB_Natural_T();
static CB_Chromatic_T POSITIONS = CB_Chromatic_T();

It is these two that don't behave as you expect them to, right? Since these are globals, you should put

extern CB_Natural_T WHITENOTES;
extern CB_Chromatic_T POSITIONS;

into a header file to declare them and

CB_Natural_T WHITENOTES;
CB_Chromatic_T POSITIONS;

into a cpp file to actually define them. The static caused these objects to have internal linkage, so every file (precisely: compilation unit) that includes the header will have two such objects created instead of sharing them between different files.

I also think these two objects are constants, right? In that case, you could declare them as such. You would then need a helper that generates these objects or a constructor that allows initializing:

CB_Natural_T whitenotes()
{
    CB_Natural_T init;
    ...
    return init;
}
CB_Natural_T const WHITENOTES = whitenotes();

Notes:

  • The "= T()" is redundant, as already mentioned.
  • The template SIZE parameter is stored in an int, which is unnecessary since the value is always present.
  • You are using a realign() function that both modifies the argument and returns the result. I'd use one of these only. Also, since it is a function that only modifies a parameter without touching any members (see point above!), you could make it a static function. At least it should be a const member function.

Upvotes: 0

Mohammed Hossain
Mohammed Hossain

Reputation: 1319

Firstly, you should not include a .cpp file. To fix the linker problem you are having, follow the inclusion model: place your function definitions in the template's header file.

Secondly, I have tried the following example program and it works now - the problem might have been due to the linker error.

Read this SO question for more information regarding including a cpp file (and templates).

main.cpp:

#include <array>
#include "circularbuffer.h"


typedef CircularBuffer<char, 7> CB_Natural_T;
typedef CircularBuffer<int, 12> CB_Chromatic_T;

static CB_Natural_T WHITENOTES = CB_Natural_T();        // buffer of letter notes
static CB_Chromatic_T POSITIONS = CB_Chromatic_T(); 

int main()
{   
    WHITENOTES.insert('C');
    WHITENOTES.insert('D');
    WHITENOTES.insert('E');
    WHITENOTES.insert('F');
    WHITENOTES.insert('G');
    WHITENOTES.insert('A');
    WHITENOTES.insert('B');

    // Initialize all positions
    for (int i = 0; i < 12; ++i)
        POSITIONS.insert(i);

    return 0;
}

circularbuffer.h:

#ifndef _CIRCULAR_BUFFER_H
#define _CIRCULAR_BUFFER_H

#include <array>

template < class T, int SIZE> 
class CircularBuffer
{
private:
    int _index;
    int _size;
    std::array<T, SIZE> _buffer;
public:
    CircularBuffer() : _index(0), _size(SIZE), _buffer() {}

    int length ()
    {
        return _size;
    }

    T& at (int index)
    {
        realign(index);
        return _buffer.at(index);
    }

    void insert (T data)
    {
        realign(_index);
        _buffer.at(_index) = data;
        _index += 1;
    }
    int index ()
    {
        return _index;
    }
private:
    int realign (int& index)
    {
        if (index >= _size)
        {
            index -= _size;
            realign(index);
        } else if (index < 0)
        {
            index += _size;
            realign(index);
        }
        return index;
    }
};

#endif

Also, use inclusion guards to make sure your files are not included twice.

Upvotes: 1

Related Questions