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.

Sign up for GitHub

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

Update coding guidelines #3785

Merged
merged 2 commits into from
May 14, 2016
Merged

Update coding guidelines #3785

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b03138c
Update coding guidelines
thojkooi May 12, 2016
7027971
Add section about params for private variables
thojkooi May 14, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 76 additions & 14 deletions wiki/development/coding-guidelines.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class ACE_Settings {
All text that will be displayed to a user shall be defined in a `stingtable.xml` file for multi-language support.

* There will not be any empty stringtable language values.
* All stringtables will follow the format as specified by Tabler and the [translation guidelines](http://ace3mod.com/wiki/development/how-to-translate-ace3.html) form .
* All stringtables will follow the format as specified by Tabler and the [translation guidelines](http://ace3mod.com/wiki/development/how-to-translate-ace3.html) form.


## 2. Macro Usage
Expand Down Expand Up @@ -110,6 +110,8 @@ These macros will call these functions with the appropriate wrappers and enable

#### 2.2.1. setVariable, getVariable family macros

These macros are allowed but are not enforced.

| Macro | Expands to |
| -------|---------|
|`GETVAR(player,MyVarName,false)` | player getVariable ["MyVarName", false]|
Expand Down Expand Up @@ -380,18 +382,18 @@ If a function returns error information, then that error information will be tes
There shall be no unreachable code.

### 6.3. Function Parameters
Parameters of functions must be retrieved through the usage of the param or params commands. If the function is part of the public API, parameters must be checked on allowed data types and values through the usage of the param and params commands.
Parameters of functions must be retrieved through the usage of the `param` or `params` commands. If the function is part of the public API, parameters must be checked on allowed data types and values through the usage of the param and params commands.

Usage of the CBA Macro `PARAM_x` or `BIS_fnc_Param` is deprecated and not allowed within the ACE project unless very specific reasons allow no other alternative.

### 6.4. Return Values
Functions and code blocks that specific a return a value must have a meaningfull return value.
Functions and code blocks that specific a return a value must have a meaningfull return value. If no meaningfull return value, the function should return nil.

### 6.5. Private Variables
All private variables shall be declared in a private array.
All private variables shall make use of the `private` keyword on initalization. When declaring a private variable before initalization, usage of the private array syntax is allowed. All private variables must be either initialized using the private key word, or declared using the private array syntax.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or by using params I guess.
params is not exclusive to the functions arguments, but is sometimes also used in forEach loops (_x params [...]), so it might be worth noting.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or any array really. It's the same as EXPLODE_N_PVT macro was.


```js
private ["_myVariable"];
private _myVariable = "hello world";
```

### 6.6. Lines of Code
Expand All @@ -400,13 +402,47 @@ Any one function shall contain no more than 250 lines of code, excluding the fun
### 6.7. Variable declarations
Declarations should be at the smallest feasible scope.


Good:
```sqf
if (call FUNC(myCondition) then {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing ) ?

private _areAllAboveTen = true; // <- smallest feasable scope

{
if (_x >= 10) then {
_areAllAboveTen = false;
};
} forEach _anArray;

if (_areAllAboveTen) then {
hint "all values are above ten!";
};
}
```

Bad:
```sqf
private _areAllAboveTen = true; // <- this is bad, because it can be initalized in the if statement
if (call FUNC(myCondition) then {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't think it was worth to mention, but this line is also missing the same closing parenthesis.

{
if (_x >= 10) then {
_areAllAboveTen = false;
};
} forEach _anArray;

if (_areAllAboveTen) then {
hint "all values are above ten!";
};
}
```

### 6.8. Variable initialization
Private variables will not be introduced until they can be initialized with meaningful values.

Good:

```js
_myVariable = 0;
private _myVariable = 0; // good because the value will be used
{
_x params ["_value", "_amount"];
if (_value > 0) then {
Expand All @@ -418,7 +454,7 @@ _myVariable = 0;
Bad:

```js
_myvariable = 0;
private _myvariable = 0; // Bad because it is initalized with a zero, but this value does not mean anything
if (_condition) then {
_myVariable = 1;
} else {
Expand All @@ -429,7 +465,7 @@ if (_condition) then {
Good:

```js
_myvariable = [1, 2] select _condition;
private _myvariable = [1, 2] select _condition;
```

### 6.9. Initialization expression in for loops
Expand All @@ -439,13 +475,12 @@ The initialization expression in a for loop will perform no actions other than t
The increment expression in a for loop will perform no action other than to change a single loop parameter to the next value for the loop.

### 6.11. GetVariable
When using getvariable, there should either be a default value given in the statement or the return value should be checked for correct data type as well as return value. A default value may not be given after a nil check.
When using getvariable, there should either be a default value given in the statement or the return value should be checked for correct data type as well as return value. A default value may not be given after a nil check.

Bad:

```js
_return = obj getvariable "varName";
if (!isnil "_return") then {_return = 0 };
if (isnil "_return") then {_return = 0 };
```

Good:
Expand All @@ -458,12 +493,37 @@ Good:

```js
_return = obj getvariable "varName";
if (!isnil "_return") exitwith {};
if (isnil "_return") exitwith {};
```

### 6.12. Global Variables
Global variables should not be used to pass along information from one function to another. Use arguments instead.

Bad:
```sqf
fnc_example = {
hint GVAR(myVariable);
};

----

GVAR(myVariable) = "hello my variable";
call fnc_example;
```

Good:
```sqf
fnc_example = {
params ["_content"];
hint _content;
};

----

["hello my variable"] call fnc_example;
```


### 6.13. Temporary Objects & Variables
Unnecessary temporary objects or variables should be avoided.

Expand Down Expand Up @@ -498,10 +558,12 @@ This is a large open source project that will get many different maintainers in
* Document that change with the reasoning in the code.

### 7.2. Scheduled vs Unscheduled
Avoid the usage of scheduled space as much as possible and stick in unscheduled. This is to provide a smooth expereince to the user by guaranteeing code to run when we want it. See Performance considerations, Spawn & ExecVm for more information.
Avoid the usage of scheduled space as much as possible and stay in unscheduled. This is to provide a smooth experience to the user by guaranteeing code to run when we want it. See Performance considerations, Spawn & ExecVm for more information.

This also helps avoid various bugs as a result of unguaranteed execution sequences when running multiple scripts.

### 7.3. Event driven
All ACE3 components shall be implemented in an event driven fashion. This is done so that code only runs when it is required and allows for modularity through low coupling components.
All ACE3 components shall be implemented in an event driven fashion. This is done to ensure code only runs when it is required and allows for modularity through low coupling components.

Event handlers in ACE3 are implemented through our event system. They should be used to trigger or allow triggering of specific functionality.

Expand Down