Author Topic: Coding style (discussion).  (Read 2722 times)

Akari

  • Moderator
  • Freak
  • *
  • Posts: 761
  • Karma: 26
    • View Profile
Coding style (discussion).
« on: 2006-12-14 09:51:04 »
I want rise topic about coding style that we use. Now because of many people working we have a lot of different styles of writing code.

Code: [Select]
WorldMap *ReadMap(const RString &path, unsigned int nb_rows, unsigned int nb_cols, unsigned int nb_sup) const;

  uint8_t *cell_data=new uint8_t[sizeof_cell];
  const unsigned int nb_cells = nb_rows * nb_cols;
  for (unsigned int c = 0; c < nb_cells; ++c)
    if (GAMEFILESYSTEM->ReadFile(path, cell_data, c * sizeof_cell, sizeof_cell))
      {
const unsigned int cell_tx = (c % nb_cols) * 4 * 8192;
const unsigned int cell_tz = (c / nb_cols) * 4 * 8192;

The only bad thing I see here are using "unsigned int" and "uint8_t" in same part of code.

My style are

Code: [Select]
    static void    DeleteScript(void);
    void           Run(FieldModule* pFieldModule, const s8 sbEntityId);
    void           Init(FieldModule* pFieldModule, const s8 sbEntityId);
    void           RequestRun(const u8 priority, const u8 scriptId);

and

Code: [Select]
Matrix
CameraManager::GetProjectionMatrix(void) const
{
    // crop screen position ro range
    s16 x = m_ssCameraPositionX;
    u16 x_add = m_usScreenWidth / 2;
    x = (x + x_add > m_ssXMax) ? m_ssXMax - x_add : x;
    x = (x - x_add < m_ssXMin) ? m_ssXMin + x_add : x;
    s16 y = m_ssCameraPositionY;
    u16 y_add = m_usScreenHeight / 2;
    y = (y + y_add > m_ssYMax) ? m_ssYMax - y_add : y;
    y = (y - y_add < m_ssYMin) ? m_ssYMin + y_add : y;

    float fMoveX = ((m_fXMax - m_fXMin) / 640.0f) * x;
    float fMoveY = ((m_fYMax - m_fYMin) / 480.0f) * y;

    return DISPLAY->GetFrustumMatrix(m_fXMin - fMoveX, m_fXMax - fMoveX, m_fYMin + fMoveY, m_fYMax + fMoveY, 1, 100000);
}

but lately I start think that using of s16 and uint8_t are bad and lowers readability of code. I think writing normal short and unsigned byte are a lot better.
This was the first thing. Second are names for class member, functions, variables and so on. We need to select one style and use it till the end and force new members to use our style.
The first thing to deside is - shal we need type of variable in variable name. For example

Code: [Select]
bool bEnabled = false;
VS
bool enabled = false;
« Last Edit: 2006-12-14 09:52:44 by Akari »

Aali

  • No life
  • *
  • Posts: 1197
  • Karma: 115
    • View Profile
Re: Coding style (discussion).
« Reply #1 on: 2006-12-14 10:57:59 »
well, you have to use u8 and friends, so you better get used to it :-P
thats what you get for writing portable software

you should probably call them u8_t or something to that effect though, that way its more obvious to the reader that its a type and not some variable name

Akari

  • Moderator
  • Freak
  • *
  • Posts: 761
  • Karma: 26
    • View Profile
Re: Coding style (discussion).
« Reply #2 on: 2006-12-14 11:53:15 »
well, you have to use u8 and friends, so you better get used to it :-P
thats what you get for writing portable software

you should probably call them u8_t or something to that effect though, that way its more obvious to the reader that its a type and not some variable name


We don't have to use it to make software portable. The size of unsigned byte written in ISO standart so it's the same everywhere.
This is used only for shorting.

Aali

  • No life
  • *
  • Posts: 1197
  • Karma: 115
    • View Profile
Re: Coding style (discussion).
« Reply #3 on: 2006-12-14 15:50:20 »
bytes, yes, but what about int, short etc? they are not the same size everywhere

einherjar

  • Fast newbie
  • *
  • Posts: 38
  • Karma: 0
    • View Profile
Re: Coding style (discussion).
« Reply #4 on: 2006-12-14 17:19:01 »
I want rise topic about coding style that we use. Now because of many
people working we have a lot of different styles of writing code.

I totally agree.

The only bad thing I see here are using "unsigned int" and "uint8_t" in same part of code.

My view here is simple:

- when one wants to use a variable with a very specific bit size, as
when reading or writing a file, or when doing low-level manipulation
(bitwsire operator, etc.) one shall use explicit integer type
(uint8_t, int16_t, etc.; I tend to prefer this style to u8 and s16 for
two reasons: (1) it is commonly used in other projects, it is
distributed by stdint.h so these types already exist on many platforms
"natively", (2) hey emacs colorize ".*_t" as a type :P )

- when one do not really care about the bitsize of the said variable,
then use the "biggest" native typenames, as int for integer, unsigned
int for integer that you know cannot be negative, and double for
floating point numbers.

I think that this particular "rule" matches what you just said:

but lately I start think that using of s16 and uint8_t are bad and
lowers readability of code. I think writing normal short and unsigned
byte are a lot better.

Regarding the second point:

names for class member, functions, variables and so on. We need to
select one style and use it till the end and force new members to use
our style.  The first thing to deside is - shal we need type of
variable in variable name. For example:

Code: [Select]
bool bEnabled = false;VS
Code: [Select]
bool enabled = false;

To my view, the type was written once, at variable declaration, and I
find this somehow redundant to "repeat" it at each variable use, and
compilers are able to track the type sufficiently correctly to warn against
an erroneous usage.

Regarding names, we will also have to choose between "case separated"
(MyVariable) and "lower-case, underscore-separated" identifiers
(my_variable). I do prefer using "case separated" identifiers for
classes, and the other style for functions/methods and variable.

Notice that this is just my current preferences, and I just followed
them as there was no "official" coding style yet. I don't know if it
may be relevant, but I follow many of the rules described here:

http://www.doc.ic.ac.uk/lab/cplus/c++.rules/

In a more general way, I think that using a "wide-used" coding style
is a good thing. Agreed, I do not have a definition for "wide", but
may be following some GNU guidlines might be good way not to choke
new developpers or potential hackers (in the good sense of the term):

http://gcc.gnu.org/onlinedocs/libstdc++/17_intro/C++STYLE

Cyberman

  • No life
  • *
  • Posts: 1575
  • Karma: 8
    • View Profile
My 2 bits
« Reply #5 on: 2006-12-14 22:24:41 »
Since my area of expertise is very low level programming in general einherjar's observation of absolute typing is important.

First the original hardware is strongly typed.

Akari, do not make the mistake of assuming a char is byte sized.  That is never specified in ANSI C. In fact it's also not specified if it's signed or not.
Depending on the compilor it can be signed or unsigned.
So explicite types are best.
I've followed similiar coding standards for int8 int16 int32 uint8 uint16 uint32 alike.
I despise the use of s8 u8 etc.  Namely because they are not EXPLICITE.  IE it doesn't say to me signed int 8 it says s8.  Whereas int8 says it's an 8 bit singed integer.  uint8 is explicite enough to say Unsigned 8 bit integer.  This is commonly accepted in embeded programing (IE tiny stuff like temperature controllers).

Because of the nature of compilors I do like the use of GetXXXX() statements. That increases portability beyond the issue of BYTE WORD DWORD and QWORD alignment within the machines memory.  I probably should over haul a lot of my software to that so that I will stop having to twiddle with the alignment in a compilor :)

short int is accepted often as 16 bit int.  This however is not always the case as some compilors define that as 32 bit int and regular int is 64bits.   (the i860's compilor did this.. it was a bit anoying at first). None of these are defined in the standard either, in fact int can be 8 16 32 64 or 128 bits. I've found all compilors are different to there encoding.  This is why, unfortunately, Q-gears needs to have explicitely defined types for reading data, because the data has specific assumed sizes and types.

Now we do not need to define the types USED by that only the types READ.  IE GetInt8() can return a type of int.  Technically it doesn't matter what the original data was therefore. However one might need to be careful with say GetInt32() because it needs to be read into at least a 32 bit int to prevent an overflow situation.  This also includes the use of floating point, doing this is a bad idea
Code: [Select]
float Temp = GetInt32();
Now for mixing int etc etc.  It's fine as long as one thing is considered before doing it:
Does the function read clearly? (IE is it obvious what is going on)
Is it necessary for explicite types to be used in parameters?
Is it necessary to return an explicite type?

If all you need to return is a signed integer using
Code: [Select]
int is fine in my view.  However when something specific is required, then you need to return that specific type. This makes clear what was important at least :D

Cyb

Akari

  • Moderator
  • Freak
  • *
  • Posts: 761
  • Karma: 26
    • View Profile
Re: Coding style (discussion).
« Reply #6 on: 2006-12-15 05:54:53 »
Since we using SDL anyway lets not make things any more difficult than it's already is, and use SDL_types.h (which includes anyway).

In SDL int types defined as follows

Code: [Select]
typedef unsigned char Uint8;
typedef signed char Sint8;
typedef unsigned short Uint16;
typedef signed short Sint16;
typedef unsigned int Uint32;
typedef signed int Sint32;

This will work corectly on every platform that SDL supports.

einherjar

  • Fast newbie
  • *
  • Posts: 38
  • Karma: 0
    • View Profile
Re: Coding style (discussion).
« Reply #7 on: 2006-12-15 18:47:52 »
Since we using SDL anyway lets not make things any more difficult than it's already is, and use SDL_types.h (which includes anyway).

In SDL int types defined as follows

Code: [Select]
typedef unsigned char Uint8;
typedef signed char Sint8;
typedef unsigned short Uint16;
typedef signed short Sint16;
typedef unsigned int Uint32;
typedef signed int Sint32;

This will work corectly on every platform that SDL supports.

Or we could also use openGL types:

Code: [Select]
GLuint, GLfloat, etc.

This would work as well on every platform that supports openGL.

Akari

  • Moderator
  • Freak
  • *
  • Posts: 761
  • Karma: 26
    • View Profile
Re: Coding style (discussion).
« Reply #8 on: 2006-12-15 20:03:59 »
Or we could also use openGL types:

Code: [Select]
GLuint, GLfloat, etc.

This would work as well on every platform that supports openGL.

OpenGl doesnt have types that shows it size in bits. GLuint less understandable than Sint32