A PropertyGrid gotcha

In a nutshell, when placed in a PropertyGrid, objects that use the same DisplayNameAttribute in different categories lead to surprising selection behavior.

I decline to call it a bug because I suspect the authors of the framework would claim this is how it's supposed to behave, but it's still the sort of thing that can trip developers up. Imagine you set a PropertyGrid's SelectedObject to an instance of this class:

class Demo
{
    [Category("Category1")]
    [DisplayName("Value")]
    public int Val1 { get; set; }

    [Category("Category2")]
    [DisplayName("Value")]
    public int Val2 { get; set; }

    [Category("Category3")]
    [DisplayName("Value")]
    public int Val3 { get; set; }
}

Sure, we're reusing the display name "Value," but since each instance is in a different category, everything should be fine, right? Well; not exactly. Under the hood, the grid determines equality between GridEntrys by comparing the combination of label, property type, and tree depth. And in this case, "label" means display name. So it doesn't matter if the actual code names of the underlying properties differ—as long as their display names, types, and depths are the same, the grid will consider their entries equal. One consequence of this is that if PropertyGrid.Refresh() is called while the Category3 Value is selected, the selection will move to the Category1 Value; presumably because it is the first entry in the tree where .Equals(oldSelection) returns true.

Luckily, there is a workaround. PropertyGrid omits tab characters when it prints display names, so you can simply add a different number of \t's to the beginning or end of each one to make them unique. A bit of a hack to be sure, but it does the job.

Thanks to stackoverflow for assisting me in my investigation of this issue.

A make_shared caveat

When you allocate a new object for a shared_ptr using the obvious approach:

std::shared_ptr<Foo> p(new Foo(a1, a2)); // case 1

you wind up with (at least) two dynamic memory allocations: one for the Foo object itself, and another for the shared_ptr's control block (which includes the strong and weak reference counts).

That's not usually a big deal, but it would help locality of reference if we could put the whole thing into one block, as well as making construction slightly faster. The make_shared library function can help us with that:

std::shared_ptr<Foo> p = std::make_shared<Foo>(a1, a2); // case 2

make_shared will typically place the shared_ptr control block and the allocated object itself into a single chunk of memory. I say "typically" because the standard does not require implementers to do things this way, but in practice, they do.

This seems like win-win, but there is a catch. Imagine a situation where the Foo object we created above is referenced by one shared_ptr and two weak_ptrs (i.e. its strong count is one and its weak count is two). Now that one shared_ptr goes away. If we'd allocated the Foo as in case 1, at this point we'd be able to free all the memory it used to occupy and go on our way. But if we'd done things as in case 2, this isn't possible. Why? Because we still need to keep the control block around until the weak count goes to zero, the control block and the Foo share a single allocation, and it isn't possible to free just part of an allocation. So while the Foo's destructor will be called the moment the strong count goes to zero as expected, if it was created using make_shared, its memory can't be returned to the system until the weak count does too. In situations where dangling weak_ptrs can hang around more or less indefinitely, this can lead to some surprising memory leaks.

pimpl makes move constructors easy

I described the pimpl idiom and some of its uses in a previous post. Another little fringe benefit is that it makes writing move constructors really, really easy. Consider this (partial) class:

class TuneResult
{
private:
    std::string m_name;
    std::map<std::pair<double, double>, double> m_measurements;
    std::pair<double, double> m_optimum;
    std::vector<std::string> m_logMessages;
};

Without pimpl, adding a move constructor requires a bit of effort:

class TuneResult
{
    TuneResult(TuneResult&& other)
        : m_name(std::move(other.m_name))
        , m_measurements(std::move(other.m_measurements))
        , m_optimum(std::move(other.m_optimum))
        , m_logMessages(std::move(other.m_logMessages))
    {    
    }

private:
    std::string m_name;
    std::map<std::pair<double, double>, double> m_measurements;
    std::pair<double, double> m_optimum;
    std::vector<std::string> m_logMessages;
};

It's not hard to imagine that becoming much longer for classes with more members. But if you've already pimpled anyway, moving becomes trivial:

// h file

class TuneResultImpl;

class TuneResult
{
    TuneResult(TuneResult&& other);
    
private:
    friend class TuneResultImpl;
    std::unique_ptr<TuneResultImpl> m_impl;
};

// cpp file 

class TuneResultImpl
{
    friend class TuneResult;

private:
    TuneResult* m_self;

    std::string m_name;
    std::map<std::pair<double, double>, double> m_measurements;
    std::pair<double, double> m_optimum;
    std::vector<std::string> m_logMessages;
};

TuneResult::TuneResult(TuneResult&& other)
    : m_impl(std::move(other.m_impl))
{
    m_impl->m_self = this; // don't forget!
}