Initialize default key contructor

simutisernestas commented 2 years ago

Addressing #118, I suppose that was intended behavior : )

aaron-skydio commented 2 years ago

This actually fails the assert in the main constructor:

  Key(const char letter, const subscript_t sub, const superscript_t super)
      : letter_(letter), sub_(sub), super_(super) {
    SYM_ASSERT(letter != kInvalidLetter);
  }

I don't think we want to remove this assert, because indeed if you're passing a letter I think we should check that it's not invalid? So I think

1) Maybe the best way to do this is to add defaults for those fields in the class, e.g. char letter_{kInvalidLetter}; 2) We should add a test case to symforce_key_test that actually calls the default constructor, since apparently none of our existing symforce tests do

simutisernestas commented 2 years ago

@aaron-skydio do you have more test case ideas ?

symforce-org / symforce

Initialize default key contructor #249