Skip to content

Instantly share code, notes, and snippets.

@rotoglup
Last active December 7, 2020 18:00
Show Gist options
  • Save rotoglup/90da8885403562a5400c4247a5638616 to your computer and use it in GitHub Desktop.
Save rotoglup/90da8885403562a5400c4247a5638616 to your computer and use it in GitHub Desktop.
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