GammaLib

The problem is related to the design of the GModel class and it’s derived classes. This is illustrated by the following code example:

#include <string>
#include <iostream>

class GModel {
public:
    GModel(void) {}
    virtual ~GModel(void) {}
    virtual std::string type(void) const = 0;
    virtual std::string name(void) const { return m_name; }
    virtual GModel& operator=(const GModel& model)
    {
        if (this != &model) {
            std::cout << "GModel& operator=(const GModel& model)" << std::endl;
            m_name = model.m_name;
        }
    }
    std::string m_name;
};

class GSkyModel : public GModel {
public:
    GSkyModel(void) { m_name="sky"; }
    virtual ~GSkyModel(void) {}
    virtual std::string type(void) const { return "sky"; }
};

class GDataModel : public GModel {
public:
    GDataModel(void) { m_name="data"; }
    virtual ~GDataModel(void) {}
    virtual std::string type(void) const { return "data"; }
};

int main(void)
{
    GModel& sky  = *(new GSkyModel);
    GModel& data = *(new GDataModel);
    std::cout << sky.type() << ":" << sky.name() << ": " << &sky << std::endl;
    if (dynamic_cast<GSkyModel*>(&sky) != NULL)
        std::cout << "is GSkyModel" << std::endl;
    else
        std::cout << "is GDataModel" << std::endl;
    sky = data;
    std::cout << sky.type() << ":" << sky.name() << ": " << &sky << std::endl;
    if (dynamic_cast<GSkyModel*>(&sky) != NULL)
        std::cout << "is GSkyModel" << std::endl;
    else                                     
        std::cout << "is GDataModel" << std::endl;
    return 0;
}

sky:sky: 0x100100080
is GSkyModel
GModel& operator=(const GModel& model)
sky:data: 0x100100080
is GSkyModel

A implementation of the main routine that has the expected behavior would be:

int main(void)
{
    GModel* sky  = new GSkyModel;
    GModel* data = new GDataModel;
    std::cout << sky->type() << ":" << sky->name() << ": " << sky << std::endl;
    if (dynamic_cast<GSkyModel*>(sky) != NULL)
        std::cout << "is GSkyModel" << std::endl;
    else
        std::cout << "is GDataModel" << std::endl;
    delete sky;
    sky = data;
    std::cout << sky->type() << ":" << sky->name() << ": " << sky << std::endl;
    if (dynamic_cast<GSkyModel*>(sky) != NULL)
        std::cout << "is GSkyModel" << std::endl;
    else
        std::cout << "is GDataModel" << std::endl;
    return 0;
}

sky:sky: 0x100100080
is GSkyModel
data:data: 0x1001000b0
is GDataModel

It is tempting to replace the reference operator by a pointer operator, yet only the reference operator allows assignment.

On the other hand, it’s the assignment that poses problems, so can we really use a reference operator for save assignment here? I would love to code an assignement such as

GModels m;
GModelPointSource p;
...
m[0] = p;

Got the solution. We need a reference to pointer operator. See the code below:

#include <string>
#include <vector>
#include <iostream>

class GModel {
public:
    GModel(void) {}
    virtual ~GModel(void) {}
    virtual std::string type(void) const = 0;
    virtual GModel* clone(void) const = 0;
    virtual std::string name(void) const { return m_name; }
    virtual GModel& operator=(const GModel& model)
    {
        if (this != &model) {
            std::cout << "GModel& operator=(const GModel& model)" << std::endl;
            m_name = model.m_name;
        }
    }
    std::string m_name;
};

class GModels {
public:
    GModels(void) {}
    virtual ~GModels(void) {}
    virtual void append(const GModel& model) { m_models.push_back(model.clone()); }
    GModel*& operator[](int index) { return (m_models[index]); }
    std::vector<GModel*> m_models;
};

class GSkyModel : public GModel {
public:
    GSkyModel(void) { m_name="sky"; }
    virtual ~GSkyModel(void) {}
    virtual std::string type(void) const { return "sky"; }
    virtual GSkyModel* clone(void) const { return new GSkyModel(*this); }
};

class GDataModel : public GModel {
public:
    GDataModel(void) { m_name="data"; }
    virtual ~GDataModel(void) {}
    virtual std::string type(void) const { return "data"; }
    virtual GDataModel* clone(void) const { return new GDataModel(*this); }
};

int main(void)
{
    GModels    models;
    GSkyModel  sky;
    GDataModel data;
    models.append(sky);
    std::cout << models[0]->type() << ":" << models[0]->name() << std::endl;
    models[0] = &data;
    std::cout << models[0]->type() << ":" << models[0]->name() << std::endl;

    return 0;
}

models[0] = data.clone();

sky:sky
data:data

Just for the record:

I implemented the Python assignment operator as a deep copy of the class. This is to make the assignment analogous to the append() method. Otherwise we would always rely on the fact that the original object does still exist. This is also analogous to the way how Python lists work;

>>> a=[]
>>> a.append(78)
>>> print a
[78]
>>> b=13
>>> a[0]=b
>>> print a
[13]
>>> del b
>>> print a
[13]

To show that the same behavior is implemented in GModels, continuing the above example we get

>>> del e
>>> print m
=== GModels ===
 Number of models ..........: 1
 Number of parameters ......: 0
=== GModelExtendedSource ===
 Name ......................: Extended source
 Instruments ...............: all
 Model type ................: 
 Number of parameters ......: 0
 Number of spatial par's ...: 0
 Number of spectral par's ..: 0
 Number of temporal par's ..: 0

Although the above solution is possible and close to the desiderate, it is subject to memory leaks. Assigning a pointer to a preexisting memory cell will overwrite the pointer that lived there before, and the link to the respective memory will be lost.

In conclusion, it is not possible to write a save method for setting values of a container class holding pointers through an access operator.

• accessing the pointers through a reference leads to slicing
• accessing the pointers through a reference to a pointer leads to memory leaks

Thus, a specific set method needs to be implemented for setting of the pointers. This should solve by the way Bug #515.

A section will be added to the coding & design standards document, explaining how this issue should be solved.