Watch out for open plan offices

 

Open plan office

 I had to work for more than a decade in an open plan office which was far from ideal. Open plan offices in the Netherlands are actual open; not with cubicles like in the USA which would give some kind of privacy. There is a lot of visual and auditory noise in the open plan office which makes concentrating and doing your work difficult. Also when you need a meeting to discuss topics you have to reserve a room instead of just talking in your own room.

 The bad part was that I anticipated all this and reported this to my manager when he came up with the initiative to create an open plan office in the new building. He shove the arguments aside and said that I should first try it out before having an opinion. I countered that the old situation was already like an open plan office so I don't need new experience. He also used the dumb argument that he himself worked happily on an open plan office so I should be happy too. He then stated that if it wouldn't work out he would reverse the situation which of course never happened.

 He then put all developers in a big open plan office but he reserved for himself a nice private room. 

 I complained about the open plan office every assessment but nothing changed. When the open plan office was finally under scrutiny the manager even lied that the whole open plan office idea was on our request and now we suddenly wanted something differently. 

 He eventually got fired; not for his open plan office but actually for the wrong reasons. Still the performance drop due the open plan office would have been a strong argument to let him go. A new manager came and put me in a private room. Other developers weren't so lucky: they still had to endure the open plan office.

Careful with refactoring

Refactoring issue

 Last year we applied a small refactoring in a piece of code. The construct was a parent - child relationship with the child hold by unique_ptr. Simplified the code was somewhat as follows:

struct Parent
{
   explicit Parent(bool bShow)
   : m_bShow(bShow)
   {
      m_ptr = std::make_unique<Child>(this);
   }

   std::unique_ptr<Child> m_ptr;
   bool                   m_bShow; 
};

struct Child
{
   explicit Child(Parent* pParent)
   : m_bShow(pParent->m_bShow)
   {
   }

   bool  m_bShow; 
};

 Although the code has a bidirectional dependency between parent and child iIt will compile if one splits out in header and source files for parent and child. 

 The heap use of child seemed redundant so it was removed and the child will be hold by value:

struct Parent
{
   explicit Parent(bool bShow)
   : m_child(this)
   , m_bShow(bShow)
   {
   }

   Child   m_child;
   bool    m_bShow; 
};

 This refactoring lead though to a bug where sometimes things were shown and sometimes not. The bug only appeared in release mode under certain conditions. It turned out that the value of 'm_bShow' was using uninitialized memory. We accidentally created a memory safety issue in years. 

 The problem is that the child is created before the parent is fully created and thereby using uninitialized memory of the parent. The solution is easy by reversing creation order:

struct Parent
{
   explicit Parent(bool bShow)
   : m_bShow(bShow)
   , m_child(this)
   {
   }

   bool    m_bShow; 
   Child   m_child;
};

 Using the 'this' pointer in constructor is a code alarm but not a code smell. As a general rule here declare first all members of built in types before declaring members with classes. If that is not viable one can defer creation by using std::optional instead of std::unique_ptr. std::optional is often more optimal from a performance perspective.

 Not sure how Rust would have prevented this; probably by disallowing the construct in the first place. That would be pity since circumventing the extra heap allocation and pointer access is certainly worthwhile the final solution where children are hold by value.