OKAY FAIL

Technical discussion for those interested in Supermodel development and Model 3 reverse engineering. Prospective contributors welcome. Not for end-user support.
Forum rules
Keep it classy!
  • No ROM requests or links.
  • Do not ask to be a play tester.
  • Do not ask about release dates.
  • No drama!
Post Reply
Ian
Posts: 39
Joined: Wed Nov 08, 2023 10:26 am

OKAY FAIL

Post by Ian »

// Boolean return codes (must be 0 or 1 only)
#define OKAY 0
#define FAIL 1

Why is this even a thing in supermodel? This seems like crazy bad design to me. If a function returns a boolean it's reasonable to expect that true would be success .. or whatever. If someone uses a boolean like a boolean

ie

bool result = Function();

if(result) {
some code;
}

They'll be in for a bad time if randomly OKAY is returned which is defined as zero lol. To fix this madness should use something like

enum class ErrorCodes { Okay, Fail };

Then bool types can't be confused.
Bart
Site Admin
Posts: 127
Joined: Tue Nov 07, 2023 5:50 am

Re: OKAY FAIL

Post by Bart »

I customarily use 0 as success (i.e., no condition) and non-zero as an error code (or just true for bool), similar to the POSIX convention, for functions that otherwise have no return type. I've seen both conventions in code bases. The reason I prefer this convention to this day over the increasingly more common true-on-success is that usually one wants to handle the error cases, not the success cases, so for me it feels really bizarre to have:

Code: Select all

if (!SaveFileToDisk(file))
{
  printf("error\n");
  return 1; // especially weird in main(), which uses my convention
}
The following has always felt more logical to me (like testing for a flagged result):

Code: Select all

if (SaveFileToDisk())
{
  printf("error\n");
  return 1;
}
I defined OKAY and FAIL to be explicit because it is just a convention after all. The intent was to be explicit about returning these rather than relying on boolean results directly (e.g. "return (foo == bar) ? OKAY : FAIL" rather than "return foo != bar".

An enum would be the better way to go in retrospect.
Ian
Posts: 39
Joined: Wed Nov 08, 2023 10:26 am

Re: OKAY FAIL

Post by Ian »

Yeah I understand the logic. In c you'd use something like errno_t which is simply a typedef for int. At least then the intention is clear. Any objections if we refactor this.
Bart
Site Admin
Posts: 127
Joined: Tue Nov 07, 2023 5:50 am

Re: OKAY FAIL

Post by Bart »

I think an enum would be good. Something like:

Code: Select all

enum Result
{
  OKAY,
  FAILED
};
Ian
Posts: 39
Joined: Wed Nov 08, 2023 10:26 am

Re: OKAY FAIL

Post by Ian »

Well I pushed the changes for this, i used enum class, then you can't accidentally mix the types, makes it more idiot proof. I didn't realise how much it was used all over the code base. Hopefully I've not broken anything.

There are a lot of classes with uninitialised variables. I know we've absolutely had bugs related to this before, would probably make sense to fix these.
Post Reply