3 ms·
I know protobuf code is extremely high quality, but I really can't stand the c-style naming conventions. I know people train themselves into grokking this and
by echelon 6mo ago
I know protobuf code is extremely high quality, but I really can't stand the c-style naming conventions.
I know people train themselves into grokking this and reading and emitting this way, but it sounds like writing "bork bork bork bork" runes to me.
I'm glad Rust feels more like Ruby and Python and that method and field names are legible.
My eyes just glaze over:
UPB_API_INLINE
const struct upb_MiniTableField* upb_MiniTable_FindFieldByNumber(
const struct upb_MiniTable* m, uint32_t number) {
const uint32_t i = number - 1; // 0 wraps to UINT32_MAX
// Ideal case: index into dense fields
if (i < m->UPB_PRIVATE(dense_below)) {
UPB_ASSERT(m->UPB_ONLYBITS(fields)[i].UPB_ONLYBITS(number) == number);
return &m->UPB_ONLYBITS(fields)[i];
}
// Early exit if the field number is out of range.
uint32_t hi = m->UPB_ONLYBITS(field_count);
uint32_t lo = m->UPB_PRIVATE(dense_below);
UPB_ASSERT(hi >= lo);
uint32_t search_len = hi - lo;
if (search_len == 0 ||
number > m->UPB_ONLYBITS(fields)[hi - 1].UPB_ONLYBITS(number)) {
return NULL;
}
// Slow case: binary search
const struct upb_MiniTableField* candidate;
#ifndef NDEBUG
candidate = UPB_PRIVATE(upb_MiniTable_ArmOptimizedLowerBound)(
m, lo, search_len, number);
UPB_ASSERT(candidate ==
UPB_PRIVATE(upb_MiniTable_LowerBound)(m, lo, search_len, number));
#elif UPB_ARM64_ASM
candidate = UPB_PRIVATE(upb_MiniTable_ArmOptimizedLowerBound)(
m, lo, search_len, number);
#else
candidate = UPB_PRIVATE(upb_MiniTable_LowerBound)(m, lo, search_len, number);
#endif
return candidate->UPB_ONLYBITS(number) == number ? candidate : NULL;
}
- ahartmetz 6mo agoI think this needs way more "upb" and "UPB" to make it clear that it is, in fact, dealing with UPBs. Whatever these are.
- jibal 6mo ago> μpb (often written 'upb') is a small protobuf implementation written in C.
- charleslmunger 6mo agoYeah namespaces and public/private would be quite nice, but C doesn't have them, so they get hacked on via macros and prefixing. The syntax was not the hard part of working or analyzing this code, though.
- teo_zero 6mo ago> I really can't stand the c-style naming conventions. Honestly I don't see much difference between upb_MiniTable_FindFieldByNumber and upb::MiniTable::FindFieldByNumber
- eximius 6mo agoThose are fairly indistinguishable. It's when they start removing letters from words to save... debug symbol bytes or something? That's when c-style naming annoys me.
- adrianton3 6mo agoIn their defence "hi" sounds very much like "high" in my mind's ear and "lo" like "low" :)
- collabs 6mo agoThis is also my pet peeve with a lot of code as well as commands like npm -g i package-name Like why would you teach people to do this? I understand people needed to save precious bytes in the sixties so we have cat and ls but saving 192 bytes or whatever with shorter variable names is not a worthwhile tradeoff anymore.
- nchmy 6mo agoWhat exactly bothers you about this and what would you prefer to see?
- collabs 6mo agoI would prefer to see full names like npm install --global @scope/PackageName At least the hi and lo can get more meaningful names. And over time we can write this in another language with private/scoped methods.