Skip to main content
Code Review

Return to Answer

Commonmark migration
Source Link

In addition to what @glampert said, I have one more suggestion:

Use a structure

#Use a structure YouYou should make structures for data that has a structure. For example, this structure shouldn't be an array of floats:

static const GLfloat triangle_vertices[] = {
 0, 1, 0,
 -1, -1, 0,
 1, -1, 0
};

Your vertices are a structure. They can be 2, 3, or 4 component vectors. You should define a structure for your vertices (and colors) that looks something like this:

typedef struct vec3 {
 GLfloat x;
 GLfloat y;
 GLfloat z;
} vec3;

Otherwise your code becomes very hard to read and much harder to maintain. You can still initialize it inline via:

static const vec3 triangle_vertices[] = {
 { 0, 1, 0 },
 { -1, -1, 0 },
 { 1, -1, 0 }
};

And if you want to do something in a loop, you can write something like:

for (int i = 0; i < MAX_VERTICES; i++)
{
 triangle_vertices [ i ].x = <whatever>;
 triangle_vertices [ i ].y = <whatever>;
 triangle_vertices [ i ].z = <whatever>;
}

instead of this other common technique:

for (int i = 0; i < MAX_VERTICES; i++)
{
 triangle_vertices [ i * 3 ] = <whatever>;
 triangle_vertices [ i * 3 + 1 ] = <whatever>;
 triangle_vertices [ i * 3 + 2 ] = <whatever>;
}

In addition to what @glampert said, I have one more suggestion:

#Use a structure You should make structures for data that has a structure. For example, this structure shouldn't be an array of floats:

static const GLfloat triangle_vertices[] = {
 0, 1, 0,
 -1, -1, 0,
 1, -1, 0
};

Your vertices are a structure. They can be 2, 3, or 4 component vectors. You should define a structure for your vertices (and colors) that looks something like this:

typedef struct vec3 {
 GLfloat x;
 GLfloat y;
 GLfloat z;
} vec3;

Otherwise your code becomes very hard to read and much harder to maintain. You can still initialize it inline via:

static const vec3 triangle_vertices[] = {
 { 0, 1, 0 },
 { -1, -1, 0 },
 { 1, -1, 0 }
};

And if you want to do something in a loop, you can write something like:

for (int i = 0; i < MAX_VERTICES; i++)
{
 triangle_vertices [ i ].x = <whatever>;
 triangle_vertices [ i ].y = <whatever>;
 triangle_vertices [ i ].z = <whatever>;
}

instead of this other common technique:

for (int i = 0; i < MAX_VERTICES; i++)
{
 triangle_vertices [ i * 3 ] = <whatever>;
 triangle_vertices [ i * 3 + 1 ] = <whatever>;
 triangle_vertices [ i * 3 + 2 ] = <whatever>;
}

In addition to what @glampert said, I have one more suggestion:

Use a structure

You should make structures for data that has a structure. For example, this structure shouldn't be an array of floats:

static const GLfloat triangle_vertices[] = {
 0, 1, 0,
 -1, -1, 0,
 1, -1, 0
};

Your vertices are a structure. They can be 2, 3, or 4 component vectors. You should define a structure for your vertices (and colors) that looks something like this:

typedef struct vec3 {
 GLfloat x;
 GLfloat y;
 GLfloat z;
} vec3;

Otherwise your code becomes very hard to read and much harder to maintain. You can still initialize it inline via:

static const vec3 triangle_vertices[] = {
 { 0, 1, 0 },
 { -1, -1, 0 },
 { 1, -1, 0 }
};

And if you want to do something in a loop, you can write something like:

for (int i = 0; i < MAX_VERTICES; i++)
{
 triangle_vertices [ i ].x = <whatever>;
 triangle_vertices [ i ].y = <whatever>;
 triangle_vertices [ i ].z = <whatever>;
}

instead of this other common technique:

for (int i = 0; i < MAX_VERTICES; i++)
{
 triangle_vertices [ i * 3 ] = <whatever>;
 triangle_vertices [ i * 3 + 1 ] = <whatever>;
 triangle_vertices [ i * 3 + 2 ] = <whatever>;
}
added 10 characters in body
Source Link
Phrancis
  • 20.5k
  • 6
  • 69
  • 155

In addition to what @glampert said, I have one more suggestion:

#Use a structure You should make structures for data that has a structure. For example, this structure shouldn't be an array of floats:

static const GLfloat triangle_vertices[] = {
 0, 1, 0,
 -1, -1, 0,
 1, -1, 0
};
static const GLfloat triangle_vertices[] = {
 0, 1, 0,
 -1, -1, 0,
 1, -1, 0
};

Your vertices are a structure. They can be 2, 3, or 4 component vectors. You should define a structure for your vertices (and colors) that looks something like this:

typedef struct vec3 {
 GLfloat x;
 GLfloat y;
 GLfloat z;
} vec3;

Otherwise your code becomes very hard to read and much harder to maintain. You can still initialize it inline via:

static const vec3 triangle_vertices[] = {
 { 0, 1, 0 },
 { -1, -1, 0 },
 { 1, -1, 0 }
};

And if you want to do something in a loop, you can write something like:

for (int i = 0; i < MAX_VERTICES; i++)
{
 triangle_vertices [ i ].x = <whatever>;
 triangle_vertices [ i ].y = <whatever>;
 triangle_vertices [ i ].z = <whatever>;
}

instead of this other common technique:

for (int i = 0; i < MAX_VERTICES; i++)
{
 triangle_vertices [ i * 3 ] = <whatever>;
 triangle_vertices [ i * 3 + 1 ] = <whatever>;
 triangle_vertices [ i * 3 + 2 ] = <whatever>;
}

In addition to what @glampert said, I have one more suggestion:

#Use a structure You should make structures for data that has a structure. For example, this structure shouldn't be an array of floats:

static const GLfloat triangle_vertices[] = {
 0, 1, 0,
 -1, -1, 0,
 1, -1, 0
};

Your vertices are a structure. They can be 2, 3, or 4 component vectors. You should define a structure for your vertices (and colors) that looks something like this:

typedef struct vec3 {
 GLfloat x;
 GLfloat y;
 GLfloat z;
} vec3;

Otherwise your code becomes very hard to read and much harder to maintain. You can still initialize it inline via:

static const vec3 triangle_vertices[] = {
 { 0, 1, 0 },
 { -1, -1, 0 },
 { 1, -1, 0 }
};

And if you want to do something in a loop, you can write something like:

for (int i = 0; i < MAX_VERTICES; i++)
{
 triangle_vertices [ i ].x = <whatever>;
 triangle_vertices [ i ].y = <whatever>;
 triangle_vertices [ i ].z = <whatever>;
}

instead of this other common technique:

for (int i = 0; i < MAX_VERTICES; i++)
{
 triangle_vertices [ i * 3 ] = <whatever>;
 triangle_vertices [ i * 3 + 1 ] = <whatever>;
 triangle_vertices [ i * 3 + 2 ] = <whatever>;
}

In addition to what @glampert said, I have one more suggestion:

#Use a structure You should make structures for data that has a structure. For example, this structure shouldn't be an array of floats:

static const GLfloat triangle_vertices[] = {
 0, 1, 0,
 -1, -1, 0,
 1, -1, 0
};

Your vertices are a structure. They can be 2, 3, or 4 component vectors. You should define a structure for your vertices (and colors) that looks something like this:

typedef struct vec3 {
 GLfloat x;
 GLfloat y;
 GLfloat z;
} vec3;

Otherwise your code becomes very hard to read and much harder to maintain. You can still initialize it inline via:

static const vec3 triangle_vertices[] = {
 { 0, 1, 0 },
 { -1, -1, 0 },
 { 1, -1, 0 }
};

And if you want to do something in a loop, you can write something like:

for (int i = 0; i < MAX_VERTICES; i++)
{
 triangle_vertices [ i ].x = <whatever>;
 triangle_vertices [ i ].y = <whatever>;
 triangle_vertices [ i ].z = <whatever>;
}

instead of this other common technique:

for (int i = 0; i < MAX_VERTICES; i++)
{
 triangle_vertices [ i * 3 ] = <whatever>;
 triangle_vertices [ i * 3 + 1 ] = <whatever>;
 triangle_vertices [ i * 3 + 2 ] = <whatever>;
}
Source Link
user1118321
  • 11.9k
  • 1
  • 20
  • 46

In addition to what @glampert said, I have one more suggestion:

#Use a structure You should make structures for data that has a structure. For example, this structure shouldn't be an array of floats:

static const GLfloat triangle_vertices[] = {
 0, 1, 0,
 -1, -1, 0,
 1, -1, 0
};

Your vertices are a structure. They can be 2, 3, or 4 component vectors. You should define a structure for your vertices (and colors) that looks something like this:

typedef struct vec3 {
 GLfloat x;
 GLfloat y;
 GLfloat z;
} vec3;

Otherwise your code becomes very hard to read and much harder to maintain. You can still initialize it inline via:

static const vec3 triangle_vertices[] = {
 { 0, 1, 0 },
 { -1, -1, 0 },
 { 1, -1, 0 }
};

And if you want to do something in a loop, you can write something like:

for (int i = 0; i < MAX_VERTICES; i++)
{
 triangle_vertices [ i ].x = <whatever>;
 triangle_vertices [ i ].y = <whatever>;
 triangle_vertices [ i ].z = <whatever>;
}

instead of this other common technique:

for (int i = 0; i < MAX_VERTICES; i++)
{
 triangle_vertices [ i * 3 ] = <whatever>;
 triangle_vertices [ i * 3 + 1 ] = <whatever>;
 triangle_vertices [ i * 3 + 2 ] = <whatever>;
}
lang-c

AltStyle によって変換されたページ (->オリジナル) /