Make it easier?

When I read the article which is a simple note for C programming,

one question was raised in my mind.

Can I make following code easier to read?

I tried several thin开发者_开发知识库gs but still it was not cool.

if(goleft)
     p->left=p->right->left;
else
     p->right=p->left->right;

Code is simple.

p is a pointer to a node.

Node is whatever you can think.

Can you show me a good answer?

It's hard to tell out of context, but calling a pointer p isn't very descriptive. node or np may be better.

(This code isn't that simple, btw; while reading it, the first thing that came to my mind was "isn't that a memory leak?")

If you feel the need for making the code more readable you can always go the objective way:

if (goleft)
  move_left(p);
else
  move_right(p);

As long as you have left and right as two explicit fields, it's hard. However, if you instead would have an array with length two, say directions, you could write something like:

  int dir = (goLeft ? 1 : 0);

  p->directions[dir] = p->directions[1 - dir]->directions[dir];

A little whitespace goes a long way:

if(goleft)
{      
  p->left = p->right->left; 
}
else
{
  p->right = p->left->right;
} 

Right now about 20 people are screaming about using {} for single statements. Let 'em scream. It gives the code some vertical breathing space, which makes it a little easier for these aging eyes to read, and it guards against the inevitable runtime bugs that arise when someone wants to add a statement to the else case (such as the one that bit me not two days ago).

That's pretty much as readable as the code is going to get (and, as readability goes, it's not that bad, at least for C). It's simple, it's explicit, anybody picking it up knows exactly what's happening. Trying to hide it behind function calls or macros for the sake of making it "prettier" is more work than it's really worth IMO.

I don't think that this actually helps in this case, but the first thing that came to my mind was something like this:

if (goleft)
{
   npp = &(p->left);
   kid = p->right->left;
}
else
{
   npp = &(p->right);
   kid = p->left->right;
}
*npp = kid;

but I really don't think that that helps at all

// a comment saying what this does
if( goleft ) p->left = p->right->left; 
else         p->right = p->left->right;

I don't know what the comment should be because this code doesn't make any sense to me; it appears to disconnect p from its left or right successor and instead points it to itself, assuming that p->right->left and p->left->right are sensibly p itself. And the test doesn't make sense because you aren't "going" anywhere, you're modifying the list. What would make sense is

Node* next = goleft? p->left : p->right;

My comment here is based on yours that this is a doubly-linked list. If instead it is a binary tree then the code makes a bit more sense, but then you're removing a node but not saving a link to it. As larsmans mentioned, this looks like a memory leak.

Some white space and a little commenting goes a long way

if (goleft)
     /* skip the immediate left turn and use the next one instead */
     p->left = p->right->left;
else
     /* skip right turn */
     p->right = p->left->right;