Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decide on basic code style guidelines #38

Closed
8 tasks done
tehKaiN opened this issue Dec 31, 2017 · 7 comments
Closed
8 tasks done

Decide on basic code style guidelines #38

tehKaiN opened this issue Dec 31, 2017 · 7 comments
Assignees
Labels
Milestone

Comments

@tehKaiN
Copy link
Member

tehKaiN commented Dec 31, 2017

Since this is multi-ppl project, a decision has to be made how code should be formatted on repo. Feel free to post suggestions below - I'll edit first post as discussion progresses.

  • Decide on braces
  • Decide on tabs vs spaces
  • Decide on folding
  • Decide on prefixes
  • Decide on include guards
  • Decide on IN/OUT
  • Apply on all source files
  • Create wiki page and/or doc file

Brace omitting - not allowed

Pros:

  • shorter code
  • improved readability for some ppl (me)

Cons:

  • expanding statement to block after control expression makes line with condition being highlighted in diff, even if condition hasn't changed - this is a major reason to write braces at all times.
  • @approxit hates it for some unknown reason

Tabs vs spaces

  • @approxit likes tab sized to 4 spaces, I like 2 spaces, so we're having source tab-based and its size set to different size in one's IDE.

Code folding

Code is horizontally limited to 80 chars, so naturally some code folding must be done.
For functions in C files:

void fnShort(t1 arg1, t2 fnNameAndArgsFitInOneLine);

void fnWithManyArgs(
  t1 arg1, t2 arg2, t3 allArgsFitInOneLineButWithoutFnName
);

void fnWithTooManyArgs(
  t1 arg1, t2 arg2, t3 thereAreTooManyFnArgsToFitInOneLine,
  t4 arg4, t5 arg5, t6 soArgListIsBrokenToMultipleLines
);

Whereas in header files:

void fnVoid(void);

void fnNonVoid(
  t1 allArgsAre,
  t2 inSeparateLine
);

Other arg formatting stuff doesn't mix with variable tab size properly. So I guess it must stay this way.

Naming conventions

camelCase with prefixes. List of prefixes:

  • ub - unsigned byte (UBYTE, uint8_t)
  • uw - unsigned word (UWORD, uint16_t)
  • ul - unsigned long (ULONG, uint32_t)
    -b, w, l for signed variants
  • p - any pointer or array
  • cb - any function pointer (callback)
  • t - typedef
  • s - struct instance
  • u - union instance
  • f - float or fixed-point var
  • undecided prefixes for double, uint64_t

for global var scoping additionally:

  • g_ - global visible from other files
  • s_ - global visible from current file (static)

Misc stuff

  • all named structs and unions should be typedefed using following convention:
typedef struct _tTypeName {

} tTypeName;
  • in header files, args should be documented using IN, OUT and INOUT defines:
void fn(
  UBYTE ubSomeInput,
  UBYTE *pSomeMoreInputDataLikeArrayOrSomething,
  UBYTE *pContentsGetOverwrittenOrArrayToBeFilled,
  UBYTE *pContentsAreUsedInFnCalculationsAndThenModified
);
  • all fns in header files must be documented using DoxyComments. Functions which are not exported in headers should be documented in .c file

  • functions should be as short as possible. If needed, split function to multiple ones. There is no hard limit for fn length, but if it doesn't fit entirely on your screen then it's a good sign you're doing something wrong.

Include guards

Currently we use something like that:

#ifndef GUARD_ACE_MANAGER_BLIT_H
#define GUARD_ACE_MANAGER_BLIT_H

Candidate for replacement:

#ifndef _ACE_MANAGERS_BLIT_H_
#define _ACE_MANAGERS_BLIT_H_

Pros:

  • not flooding intellisense starting with "G"
  • reflects filesystem location: source file in inc/ace/managers/blit.h
@tehKaiN
Copy link
Member Author

tehKaiN commented Jan 21, 2018

Since we're doxycommenting each function, I doubt there is need to use IN/OUT/INOUT attributes anymore. Removing them and allowing putting more than one arg in function declaration would decrease header size - currently blit.h is ugly as hell.

@tehKaiN
Copy link
Member Author

tehKaiN commented Mar 16, 2018

I've updated brace omission to not allowed, since diff reasons are indisputable

@tehKaiN tehKaiN added this to the Meta milestone Apr 28, 2018
@tehKaiN
Copy link
Member Author

tehKaiN commented Apr 28, 2018

I'd really drop IN/OUT attributes since they are not that standard, intellisense skips them and vscode doxycomments plugin which I use wrongly suggests them as var type and proper type as var name.

@approxit
Copy link
Collaborator

approxit commented May 1, 2018

Is there any alternative way to annotate function argument as IN/OUT in Doxygen?
Or we are going to go back to consts?

@tehKaiN
Copy link
Member Author

tehKaiN commented May 2, 2018

Not really. Most common practice for input ptrs is to declare them as const ptr.

@tehKaiN
Copy link
Member Author

tehKaiN commented May 4, 2018

I've added include guards replacement candidate. @approxit let me know what you think.

@approxit
Copy link
Collaborator

approxit commented May 4, 2018

+1 on guards from me and +1 on ditching IN/OUT/INOUTs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants