Skip to content

Instantly share code, notes, and snippets.

Embed
What would you like to do?
Code refactor, thoughts

Some thoughts about code structure, as I read code, and find some things some hard to follow than others.

Booleans are values

(m_ui.SomeCheckBox->checkState() == Qt::CheckState::Checked) ? true : false

Should probably just be

(m_ui.SomeCheckBox->checkState() == Qt::CheckState::Checked)

Also

if (canRemoveItem)
{
  m_ui.RemoveBtn->setEnabled(true);
}
else
{
  m_ui.RemoveBtn->setEnabled(false);
}
m_ui.EditBtn->setEnabled(true);
m_ui.DuplicateBtn->setEnabled(true);

Should be

m_ui.RemoveBtn->setEnabled(canRemoveItem);
m_ui.EditBtn->setEnabled(true);
m_ui.DuplicateBtn->setEnabled(true);

Control flow, keep common cases together

if (canRemoveItem)
{
  m_ui.RemoveBtn->setEnabled(true);
  m_ui.EditBtn->setEnabled(true);
  m_ui.DuplicateBtn->setEnabled(true);
}
else
{
  m_ui.RemoveBtn->setEnabled(false);
  m_ui.EditBtn->setEnabled(true);
  m_ui.DuplicateBtn->setEnabled(true);
}

Should be

if (canRemoveItem)
{
  m_ui.RemoveBtn->setEnabled(true);
}
else
{
  m_ui.RemoveBtn->setEnabled(false);
}
m_ui.EditBtn->setEnabled(true);
m_ui.DuplicateBtn->setEnabled(true);

Constants should have names

// From the most significant bit to the least : coordinates(256), ..., name(8), identifier(4), ..., index(1)
// Mask value for index and identifier and name is 13 (8+4+1)
uint32_t displayObjectsMask = 13;

Should probably be

const uint32_t BIT_MASK_COORDINATES = (1u<<8);
...
const uint32_t BIT_MASK_NAME        = (1u<<3);
const uint32_t BIT_MASK_IDENTIFIER  = (1u<<2);
const uint32_t BIT_MASK_INDEX       = (1u<<0);
uint32_t displayObjectsMask = BIT_MASK_NAME | BIT_MASK_IDENTIFIER | BIT_MASK_INDEX;
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment