Jump to content

  • Log In with Google      Sign In   
  • Create Account


Academia

  • You cannot reply to this topic
36 replies to this topic

#1 Nercury   Crossbones+   -  Reputation: 766

Like
13Likes
Like

Posted 13 April 2013 - 09:10 AM

Academia. Full of brilliant people. However, more often than not, when it comes to programming, they produce the hardest code to read EVER.
 
Why do you need to name your class fields as c, cc, d, e? What the hell are you trying to obfuscate? And no, that wall of well-aligned comments is not a solution. And please, stop teaching students variables by naming them a, b, c, d. Please. Just. Stop. Later they become professors who can not produce a readable code.
 
You probably think that your code looks good because it nicely fits in a box. I know that you were making pixel shaders before I was even born, but you are wrong.
 
Another example:
/** A two dimensional array holding particle positions. For the
 * derived container_poly class, this also holds particle
 * radii. */
double **p;
 
 
Why don't you just name it "positions"? What are you trying to save? Certainly not a compiled binary size, because the name does not matter there. Are you producing that extremely complicated code inside this class SO FAST that you can not type a full name of "positions" variable every time it is needed? No wonder you end up with your method contents looking like this:
 
 
 
int l,dijk=di+nx*(dj+oy*dk),dijkl,dijkr,ima=step_div(dk-ez,nz);
int qj=dj+step_int(-ima*byz*ysp),qjdiv=step_div(qj-ey,ny);
int qi=di+step_int((-ima*bxz-qjdiv*bxy)*xsp),qidiv=step_div(qi,nx);
int fi=qi-qidiv*nx,fj=qj-qjdiv*ny,fijk=fi+nx*(fj+oy*(dk-ima*nz)),fijk2;
double disy=ima*byz+qjdiv*by,switchy=(dj-ey)*boxy-ima*byz-qjdiv*by;
double disx=ima*bxz+qjdiv*bxy+qidiv*bx,switchx=di*boxx-ima*bxz-qjdiv*bxy-qidiv*bx;
double switchx2,disxl,disxr,disx2,disxr2;

/*
so you have created qj, qi, and more badly named
crap at the top of your function... again.
*/
 
 
Oh sure it is more compact. And that is NOT a good thing. Of COURSE I can figure out what it all means eventually, I am not THAT stupid. But it is not the point.
 
 
What if you have another friggin' class where "p" means "pizza"? Why do you have to remember to mentally switch from "oh, I am working with particle class so "p" means "positions"" to "oh this is pizza delivery class so "p" means "pizza"".
 
But yeah, I understand that you won't encounter "pizza" anywhere, because you have never worked with a huge code base where you simply can not remember everything.
 
P.S. This little rant was not intended as a good critique. It just sometimes gets to me smile.png

Edited by Nercury, 13 April 2013 - 09:11 AM.


Sponsor:

#2 Paradigm Shifter   Crossbones+   -  Reputation: 5257

Like
0Likes
Like

Posted 13 April 2013 - 09:17 AM

Yeah, you need longer variable names. I suggest:

 

alpha instead of a

beta instead of b

pi_thing instead of p (since pi may cause confusion).


"Most people think, great God will come from the sky, take away everything, and make everybody feel high" - Bob Marley

#3 slicer4ever   Crossbones+   -  Reputation: 3519

Like
0Likes
Like

Posted 13 April 2013 - 10:41 AM


int l,dijk=di+nx*(dj+oy*dk),dijkl,dijkr,ima=step_div(dk-ez,nz);
int qj=dj+step_int(-ima*byz*ysp),qjdiv=step_div(qj-ey,ny);
int qi=di+step_int((-ima*bxz-qjdiv*bxy)*xsp),qidiv=step_div(qi,nx);
int fi=qi-qidiv*nx,fj=qj-qjdiv*ny,fijk=fi+nx*(fj+oy*(dk-ima*nz)),fijk2;
double disy=ima*byz+qjdiv*by,switchy=(dj-ey)*boxy-ima*byz-qjdiv*by;
double disx=ima*bxz+qjdiv*bxy+qidiv*bx,switchx=di*boxx-ima*bxz-qjdiv*bxy-qidiv*bx;
double switchx2,disxl,disxr,disx2,disxr2;

/*
so you have created qj, qi, and more badly named
crap at the top of your function... again.
*/


O man, that's just....I don't even know. is their actually comma separated code in that? and a random variable doing...nothing on the 4th line?
that student needs to get a talking to.

I think with things like visual assist x, and code-completion tabbing. i've pretty much stopped doing this practice(although i never did it very much anyway). I like to use full names for things most of the time, just because of the fact that it looks nicer at the end of the day.

edit: that's actually making me a bit sick looking at it, i'm ganna go lie down for a bit now.

Edited by slicer4ever, 13 April 2013 - 10:47 AM.

Check out https://www.facebook.com/LiquidGames for some great games made by me on the Playstation Mobile market.

#4 Paradigm Shifter   Crossbones+   -  Reputation: 5257

Like
0Likes
Like

Posted 13 April 2013 - 10:42 AM

int l;

is especially nice, especially if you use a font which makes l and 1 look very similar.

 

EDIT: Obviously it should be called lambda


Edited by Paradigm Shifter, 13 April 2013 - 10:43 AM.

"Most people think, great God will come from the sky, take away everything, and make everybody feel high" - Bob Marley

#5 Nercury   Crossbones+   -  Reputation: 766

Like
0Likes
Like

Posted 13 April 2013 - 11:35 AM

a random variable doing...nothing on the 4th line?

 

Well to be fair I have hand-picked the worst from this code, and this is just a beginning of the method.



#6 Sik_the_hedgehog   Crossbones+   -  Reputation: 1609

Like
1Likes
Like

Posted 13 April 2013 - 05:58 PM

Oh sure it is more compact. And that is NOT a good thing. Of COURSE I can figure out what it all means eventually, I am not THAT stupid. But it is not the point.

I can't and I'm not that stupid (OK, I am, but still). That thing is just plain unreadable, I have an easier time reading blobs from hex editors... I'd kick immediately anybody who came up with code like that. The only place where I'd tolerate something like that is for stuff like 4k demoscene compos or something similar (and where the 4k limit applies on the source code, e.g. javascript demos and such).

 

Also oh god that last link... WHY (╯°□°)╯︵ ┻━┻


Don't pay much attention to "the hedgehog" in my nick, it's just because "Sik" was already taken =/ By the way, Sik is pronounced like seek, not like sick.

#7 Bacterius   Crossbones+   -  Reputation: 8584

Like
2Likes
Like

Posted 13 April 2013 - 06:10 PM

remap(ai,aj,ak,ci,cj,ck,x,y,z,ijk);
vc.find_voronoi_cell(x,y,z,ci,cj,ck,ijk,w,mrs);
This code has to be generated by a machine. Or he did a search & replace at the very end. No sane human can make sense of this.

The slowsort algorithm is a perfect illustration of the multiply and surrender paradigm, which is perhaps the single most important paradigm in the development of reluctant algorithms. The basic multiply and surrender strategy consists in replacing the problem at hand by two or more subproblems, each slightly simpler than the original, and continue multiplying subproblems and subsubproblems recursively in this fashion as long as possible. At some point the subproblems will all become so simple that their solution can no longer be postponed, and we will have to surrender. Experience shows that, in most cases, by the time this point is reached the total work will be substantially higher than what could have been wasted by a more direct approach.

 

- Pessimal Algorithms and Simplexity Analysis


#8 Chad Smith   Members   -  Reputation: 1110

Like
3Likes
Like

Posted 14 April 2013 - 12:52 AM

int l,dijk=di+nx*(dj+oy*dk),dijkl,dijkr,ima=step_div(dk-ez,nz);
int qj=dj+step_int(-ima*byz*ysp),qjdiv=step_div(qj-ey,ny);
int qi=di+step_int((-ima*bxz-qjdiv*bxy)*xsp),qidiv=step_div(qi,nx);
int fi=qi-qidiv*nx,fj=qj-qjdiv*ny,fijk=fi+nx*(fj+oy*(dk-ima*nz)),fijk2;
double disy=ima*byz+qjdiv*by,switchy=(dj-ey)*boxy-ima*byz-qjdiv*by;
double disx=ima*bxz+qjdiv*bxy+qidiv*bx,switchx=di*boxx-ima*bxz-qjdiv*bxy-qidiv*bx;
double switchx2,disxl,disxr,disx2,disxr2;

/*
so you have created qj, qi, and more badly named
crap at the top of your function... again.
*/

What the?  What human could just look at that and know exactly what is going on?  If I had to read crap like that every day I'd just quit programming.  I don't think it'd be worth the pain every day.



#9 Olof Hedman   Crossbones+   -  Reputation: 2758

Like
4Likes
Like

Posted 14 April 2013 - 02:38 AM

Makes me remember my best friend in highschool.

I don't think I ever say him use a variable name with more then 4 characters. mostly 3.

 

I guess it looked good together with the embedded asm he was writing.

 

He also didn't like to use circuit boards when building his electronics, because they were a waste of time and space.

Instead he build his circuits with all air-connections, in 3D... (yes, they did work, perfectly)

 

He's now pretty far into his academic career :P



#10 RobTheBloke   Crossbones+   -  Reputation: 2336

Like
7Likes
Like

Posted 14 April 2013 - 04:41 AM

I was a demonstrator / lecturer for a few years, and had to demonstrate for one particularly terrible lecturer. Most of his code was so far beyond bad it simply wasn't funny. The one code example he gave to students that sticks out most, was his OBJ loading & rendering code. I've ommitted support for the normals and UVs, but I think you get the idea.....

struct Vertex
{
   int index;
   float x;
   float y;
   float z;
   Vertex* next;
};
float getX(Vertex* head, int index)
{
   Vertex* p = head;
   while(p)
   {
      if(p->index == index)
         return p->x;
      p = p->next;
   }
   return 0;
}
float getY(Vertex* head, int index)
{
   Vertex* p = head;
   while(p)
   {
      if(p->index == index)
         return p->y;
      p = p->next;
   }
   return 0;
}
float getZ(Vertex* head, int index)
{
   Vertex* p = head;
   while(p)
   {
      if(p->index == index)
         return p->z;
      p = p->next;
   }
   return 0;
}
struct Face
{
   int index;
   int i0;
   int i1;
   int i2;
   Face* next;
};
int getV0(Face* head, int index)
{
   Face* f = head;
   while(f)
   {
      if(f->index == index)
         return f->i0;
      f = f->next;
   }
   return 0;
}
int getV1(Face* head, int index)
{
   Face* f = head;
   while(f)
   {
      if(f->index == index)
         return f->i1;
      f = f->next;
   }
   return 0;
}
int getV2(Face* head, int index)
{
   Face* f = head;
   while(f)
   {
      if(f->index == index)
         return f->i2;
      f = f->next;
   }
   return 0;
}
void drawMesh(Face* f, Vertex* v, int n)
{
   glBegin(GL_TRIANGLES);
   for(int i=0;i<n;++i)
   {
       glVertex3f(getX(getV0(f,i)), getY(getV0(f,i)), getZ(getV0(f,i)));
       glVertex3f(getX(getV1(f,i)), getY(getV1(f,i)), getZ(getV1(f,i)));
       glVertex3f(getX(getV2(f,i)), getY(getV2(f,i)), getZ(getV2(f,i)));
   }
   glEnd();
}


#11 slicer4ever   Crossbones+   -  Reputation: 3519

Like
1Likes
Like

Posted 14 April 2013 - 04:47 AM


struct Vertex
{
   int index;
   float x;
   float y;
   float z;
   Vertex* next;
};
float getX(Vertex* head, int index)
{
   Vertex* p = head;
   while(p)
   {
      if(p->index == index)
         return p->x;
      p = p->next;
   }
   return 0;
}
float getY(Vertex* head, int index)
{
   Vertex* p = head;
   while(p)
   {
      if(p->index == index)
         return p->y;
      p = p->next;
   }
   return 0;
}
float getZ(Vertex* head, int index)
{
   Vertex* p = head;
   while(p)
   {
      if(p->index == index)
         return p->z;
      p = p->next;
   }
   return 0;
}
struct Face
{
   int index;
   int i0;
   int i1;
   int i2;
   Face* next;
};
int getV0(Face* head, int index)
{
   Face* f = head;
   while(f)
   {
      if(f->index == index)
         return f->i0;
      f = f->next;
   }
   return 0;
}
int getV1(Face* head, int index)
{
   Face* f = head;
   while(f)
   {
      if(f->index == index)
         return f->i1;
      f = f->next;
   }
   return 0;
}
int getV2(Face* head, int index)
{
   Face* f = head;
   while(f)
   {
      if(f->index == index)
         return f->i2;
      f = f->next;
   }
   return 0;
}
void drawMesh(Face* f, Vertex* v, int n)
{
   glBegin(GL_TRIANGLES);
   for(int i=0;i<n;++i)
   {
       glVertex3f(getX(getV0(f,i)), getY(getV0(f,i)), getZ(getV0(f,i)));
       glVertex3f(getX(getV1(f,i)), getY(getV1(f,i)), getZ(getV1(f,i)));
       glVertex3f(getX(getV2(f,i)), getY(getV2(f,i)), getZ(getV2(f,i)));
   }
   glEnd();
}


o god, that's horrid, and so incredibly inefficient, I don't even know.
Check out https://www.facebook.com/LiquidGames for some great games made by me on the Playstation Mobile market.

#12 RobTheBloke   Crossbones+   -  Reputation: 2336

Like
2Likes
Like

Posted 14 April 2013 - 05:17 AM

o god, that's horrid, and so incredibly inefficient, I don't even know.

It was all like that, but his exam marking was his worst offence. He'd often fail 50% of a year group, and barely pass the rest. All because he had absolutely no clue. If the students had me as a second marker, they were lucky. Some other 2nd markers weren't as diligant though :(
 

Why do you need to name your class fields as c, cc, d, e? What the hell are you trying to obfuscate?

 

Most lecturers are generally pretty level headed, and do try to keep their code clean and readable. The more research based they are however, the more chance there is for strangeness to creep in. Academic research can be quite an isolating experience, so you don't often have people around to bounce ideas off, which does tend to encourage people to slip into some really bizarre habits.

 

There is also a tendancy for some senior academics to stick their name on top any paper from any student they are supervising. This experience can make a junior researcher quite paranoid, and overly protective of their work. Once they've got a Phd, often this paranoia doesn't leave the academic, but simply becomes much more engrained. Maintaining secrecy, and deliberately obfuscating code, is not uncommon in the academic world as a result..... 



#13 Sik_the_hedgehog   Crossbones+   -  Reputation: 1609

Like
3Likes
Like

Posted 14 April 2013 - 11:12 PM

I also imagine that for those with a heavy maths background they will try to name program variables as if they were doing so in an equation out of custom (since in equations everything is a letter and maybe a subscript).


Don't pay much attention to "the hedgehog" in my nick, it's just because "Sik" was already taken =/ By the way, Sik is pronounced like seek, not like sick.

#14 Bacterius   Crossbones+   -  Reputation: 8584

Like
0Likes
Like

Posted 14 April 2013 - 11:24 PM

I also imagine that for those with a heavy maths background they will try to name program variables as if they were doing so in an equation out of custom (since in equations everything is a letter and maybe a subscript).

 

True, but in general equation variables are named sensibly. I have trouble conceiving an equation mapping to this:

 

step_int((-ima*bxz-qjdiv*bxy)*xsp)
 

 

Nah it just looks like someone trying to use 1-letter variables, running out, moving on to 2-letter variables, running out, and so on.

 

Reasonable variable names are descriptive stuff like "delta", "x_prime", "du", and so on. But having multiple variables called "disx", "disy", "disxl", "disxr", "disx2", "disxr2" is unreasonable. It's insane. You have to look up their meaning to invoke them when needed, unless you know the code by heart (which should not be the case for even remotely self-documenting code). It might be good for a code-and-forget kind of project or library, but good luck maintaining this.

 

PS: or excessive precomputation, maybe.


Edited by Bacterius, 14 April 2013 - 11:29 PM.

The slowsort algorithm is a perfect illustration of the multiply and surrender paradigm, which is perhaps the single most important paradigm in the development of reluctant algorithms. The basic multiply and surrender strategy consists in replacing the problem at hand by two or more subproblems, each slightly simpler than the original, and continue multiplying subproblems and subsubproblems recursively in this fashion as long as possible. At some point the subproblems will all become so simple that their solution can no longer be postponed, and we will have to surrender. Experience shows that, in most cases, by the time this point is reached the total work will be substantially higher than what could have been wasted by a more direct approach.

 

- Pessimal Algorithms and Simplexity Analysis


#15 Khatharr   Crossbones+   -  Reputation: 2964

Like
0Likes
Like

Posted 14 April 2013 - 11:50 PM

"du"?


void hurrrrrrrr() {__asm sub [ebp+4],5;}

There are ten kinds of people in this world: those who understand binary and those who don't.

#16 Bacterius   Crossbones+   -  Reputation: 8584

Like
0Likes
Like

Posted 14 April 2013 - 11:54 PM

"du"?

 

Change in the variable "u". For a physics library I can see that coming up and people would know what it is referring to (if the "u" variable is clear, of course). Maybe "dt" is a better example (change in time, and the ubiquitous update(double dt) function signature)


Edited by Bacterius, 14 April 2013 - 11:58 PM.

The slowsort algorithm is a perfect illustration of the multiply and surrender paradigm, which is perhaps the single most important paradigm in the development of reluctant algorithms. The basic multiply and surrender strategy consists in replacing the problem at hand by two or more subproblems, each slightly simpler than the original, and continue multiplying subproblems and subsubproblems recursively in this fashion as long as possible. At some point the subproblems will all become so simple that their solution can no longer be postponed, and we will have to surrender. Experience shows that, in most cases, by the time this point is reached the total work will be substantially higher than what could have been wasted by a more direct approach.

 

- Pessimal Algorithms and Simplexity Analysis


#17 Khatharr   Crossbones+   -  Reputation: 2964

Like
0Likes
Like

Posted 15 April 2013 - 05:20 AM

Oh, you mean delta_u.


void hurrrrrrrr() {__asm sub [ebp+4],5;}

There are ten kinds of people in this world: those who understand binary and those who don't.

#18 Bacterius   Crossbones+   -  Reputation: 8584

Like
3Likes
Like

Posted 15 April 2013 - 05:35 AM

Oh, you mean delta_u.

 

Nah, too long, "du" is used everywhere in mathematics as notation so it's a perfect candidate as a variable name. On the other hand, more complicated rates of change could use better variable names, "d3udv3" doesn't work out so well happy.png


The slowsort algorithm is a perfect illustration of the multiply and surrender paradigm, which is perhaps the single most important paradigm in the development of reluctant algorithms. The basic multiply and surrender strategy consists in replacing the problem at hand by two or more subproblems, each slightly simpler than the original, and continue multiplying subproblems and subsubproblems recursively in this fashion as long as possible. At some point the subproblems will all become so simple that their solution can no longer be postponed, and we will have to surrender. Experience shows that, in most cases, by the time this point is reached the total work will be substantially higher than what could have been wasted by a more direct approach.

 

- Pessimal Algorithms and Simplexity Analysis


#19 Krohm   Crossbones+   -  Reputation: 3052

Like
0Likes
Like

Posted 15 April 2013 - 06:33 AM

I fully endorse this product and/or service.



#20 Tom KQT   Members   -  Reputation: 1564

Like
1Likes
Like

Posted 15 April 2013 - 11:55 AM

I wouldn't say that using p as a name for verticle position variable is a coding horror, especially if it is some particle class with position (p), velocity (v) and acceleration (a), where the meaning is quite obvious. I personaly would use at least pos, vel, acc, but probably not the full text. Acceleration is so long to write...







PARTNERS