Установка бит и читаемость кода

StackOverflow https://stackoverflow.com/questions/1197111

Вопрос

У меня есть приложение Arduino (ну, на самом деле библиотека), в котором есть несколько флагов состояния, и изначально я просто объявил их как целые числа (ну, uint8_t, в данном случае 8-битные беззнаковые символы).Но я мог бы объединить их все в одно целое число и использовать операции с битовой маской для установки и проверки статуса.

Пример первого:

if (_shift == HIGH)
{
    _shift = LOW;
}
else
{
    _shift = HIGH;
}

пример последнего

#define SHIFT_BIT 0

if (bitRead(_flags, SHIFT_BIT) == HIGH)
{
   bitWrite(_flags, SHIFT_BIT, LOW);
}
else
{
   bitWrite(_flags, SHIFT_BIT, HIGH);
}

Первый читается лучше, но второй более эффективен (пространство и время).Должна ли в этой ситуации всегда побеждать эффективность пространства и времени или это своего рода оптимизация, которая должна происходить только тогда, когда это необходимо?

(Добавлен)

Для полноты вот определение проводки этих bitWrite и т. д.макросы:

#define bitRead(value, bit) (((value) >> (bit)) & 0x01)
#define bitSet(value, bit) ((value) |= (1UL << (bit)))
#define bitClear(value, bit) ((value) &= ~(1UL << (bit)))
#define bitWrite(value, bit, bitvalue) (bitvalue ? bitSet(value, bit) : bitClear(value, bit))
Это было полезно?

Решение

Проверить Отличный дубль Рэймонда Чена по этому вопросу.Таким образом, вам нужно провести детальный расчет, чтобы выяснить, действительно ли последний случай более эффективен в зависимости от количества объектов по сравнению с обычным.сколько сайтов вызова на самом деле устанавливают эти состояния.

Что касается читаемости, похоже, что вы делаете это с переменными-членами, а это значит, что вы, вероятно, инкапсулировали их в хорошие функции.В этом случае меня не так волнует читабельность, потому что, по крайней мере, код для людей, использующих этот класс, выглядит хорошо.Однако вы всегда можете инкапсулировать его в приватные функции, если это вас беспокоит.

Другие советы

В зависимости от совместимости компилятора AVR-GCC, в чем я не уверен, вы можете сделать что-то подобное и сохранить порядок и чистоту.

struct flags {
    unsigned int flag1 : 1;  //1 sets the length of the field in bits
    unsigned int flag2 : 4;
}; 

flags data;

data.flag1 = 0;
data.flag2 = 12;

if (data.flag1 == 1)
{
    data.flag1 = 0;
}
else
{
    data.flag1 = 1;
}

Если вы также хотите получить доступ ко всему флагу int сразу, то:

union 
{
    struct {
        unsigned int flag1 : 1;  //1 sets the length of the field in bits
        unsigned int flag2 : 4;
    } bits;
    unsigned int val;
} flags;

Затем вы можете получить доступ к биту с помощью двух уровней косвенности: variable.bits.flag1<--возвращает однобитовый флаг или один уровень, чтобы получить все целое число флагов: variable.val <--возвращает целое число

Будет понятнее, если убрать необходимость использования констант. HIGH и LOW, разделив его на два метода.Просто сделай bitSet и bitClear методы. bitSet устанавливает бит в HIGH, и bitClear устанавливает бит в LOW.Тогда становится:

#define SHIFT_BIT 0

if (bitRead(_flags, SHIFT_BIT) == HIGH)
{
    bitClear(_flags, SHIFT_BIT);
}
else
{
    bitSet(_flags, SHIFT_BIT);
}

И, конечно, если у вас просто есть HIGH == 1 и LOW == 0, то вам не нужна проверка ==.

На мой взгляд, даже ваш последний код все еще вполне читаем.Дав имя каждому из ваших флагов, код можно будет прочитать без особых усилий.

Плохой способ сделать это — использовать «магические» числа:

if( _flags | 0x20 ) {  // What does this number mean?
   do_something();
}

Если вам не нужно оптимизировать, не делайте этого и используйте самое простое решение.

Если вам все же нужна оптимизация, вы должны знать, для чего:

  • Первая версия была бы минимально быстрее, если бы вы только устанавливали или очищали бит, а не переключали его, потому что тогда вам не нужно читать память.

  • Первая версия лучше.параллелизм.Во втором у вас есть чтение-изменение-запись, поэтому вам нужно убедиться, что к байту памяти не осуществляется одновременный доступ.Обычно вы отключаете прерывания, что несколько увеличивает задержку прерывания.Кроме того, если забыть отключить прерывания, это может привести к очень неприятным и трудно обнаруживаемым ошибкам (самая неприятная ошибка, с которой я столкнулся до сих пор, была именно такого рода).

  • Первая версия минимально лучше по сравнению с другими.размер кода (меньше использования флэш-памяти), поскольку каждый доступ представляет собой одну операцию загрузки или сохранения.Второй подход требует дополнительных битовых операций.

  • Вторая версия использует меньше оперативной памяти, особенно если у вас этих бит много.

  • Вторая версия также работает быстрее, если вы хотите протестировать несколько битов одновременно (например,является одним из установленных битов).

Если вы говорите о читабельности, битовых наборах и C++, то почему я ничего не нашел на std::bitset там?Я понимаю, что раса встроенных программистов вполне устраивает битовые маски и ослепла из-за их явного уродства (маски, а не расы:), но помимо масок и битовых полей в стандартной библиотеке есть и довольно элегантное решение.

Пример:

#include <bitset>

enum tFlags { c_firstflag, c_secondflag, c_NumberOfFlags };

...

std::bitset<c_NumberOfFlags> bits;

bits.set( c_firstflag );
if( bits.test( c_secondflag ) ) {
  bits.clear();
}

// even has a pretty print function!
std::cout << bits << std::endl;// does a "100101" representation.

Для битовых полей лучше использовать логические операции, поэтому вы можете сделать:

if (flags & FLAG_SHIFT) {
   flags &= ~FLAG_SHIFT;
} else {
   flags |= FLAG_SHIFT;
}

Теперь он имеет внешний вид первого и эффективность второго.Теперь у вас могут быть макросы, а не функции, поэтому (если я правильно понял, это будет что-то вроде):

#define bitIsSet(flags,bit) flags | bit
#define bitSet(flags,bit) flags |= bit
#define bitClear(flags,bit) flags &= ~bit

У вас нет накладных расходов на вызов функций, и код снова становится более читабельным.

Я не играл с Arduino (пока), но, возможно, уже есть макросы для подобных вещей, я не знаю.

Я бы сказал, что первое, что меня беспокоит, это:«#Define Shift 0» Почему бы не использовать постоянный, а не макрос?Что касается эффективности, константа позволяет определить тип, тем самым гарантируя, что впоследствии преобразование не потребуется.

Что касается эффективности вашей техники:- Во -первых, избавьтесь от предложения Else (зачем установить бит на высокий уровень, если его значение уже высокое?) - во -вторых, предпочитаете сначала что -то читаемое, вставленные сеттеры / получатели, использующие бит внутри страны, будут выполнять работу, быть эффективными и быть читаемым.

Что касается хранилища, то для C++ я бы предпочел использовать набор битов (в сочетании с перечислением).

Не слишком ли просто просто сказать:

flags ^= bit;
Лицензировано под: CC-BY-SA с атрибуция
Не связан с StackOverflow
scroll top