Code:#include <stdio.h>
#include <stdlib.h>
void fun(struct node **x, struct node **y){
struct node *k, *l;
k=&(*x);
l=&(*y);
if(k->data>l->data)
printf("true");
else
printf("false");
}
struct node(){
int data;
};
void main(){
struct *i, *j;
int p=34, q=89;
i=(struct i)malloc(sizeof(struct node));
i->data=p;
j=(struct i)malloc(sizeof(struct node));
j->data=q;
fun(&i,&j);
getch();
}
If you make an effort to understand the error messages from your compiler, you will learn more than you would if someone simply summarised the (several) things wrong with your program.
Well, for starters, you should tell us what compiler warnings/errors are you receiving.
I got quite a list from your code; here's the first two:
This is saying your struct declaration should go up top, before it is referenced in any code.Code:/*
main.c|3|warning: 'struct node' declared inside parameter list|
main.c|3|warning: its scope is only this definition or declaration, which is probably not what you want|
*/
Based on the various warnings and errors I received trying to compile your code, I get the impression you pretty much typed out all the code and only tried compiling it at the end.
I'd recommend you scrap what you have and start over, but this time, only write a little bit of code, compile, fix any warnings/errors, and test. Then add a little more code, compile, fix any warnings/errors, and test. Build up your program gradually, compiling and testing as you go.
I would suggested starting with only your struct above "main()", and in "main()", declare a struct variable, give it a value, and print it out. Compile and test this. When it works, then add a bit more code, following the advice in the previous paragraph.
Also, the return type of "main()" should be int, not void. One of the proper forms of "main()" is as follows:
Link to FAQ: FAQ > main() / void main() / int main() / int main(void) / int main(int argc, char *argv[]) - Cprogramming.comCode:int main(void)
{
//
return 0;
}
And "getch()" is a non-standard function. Consider using "getchar()", or using other methods to keep the console window open.
Link to FAQ: FAQ > Stop my Windows Console from disappearing everytime I run my program? - Cprogramming.com
OMG. One of the ugliest programs I've seen in my life.
Code:#include <stdio.h> // puts
#include <stdlib.h> // malloc
struct node {
int data;
};
int fun(const struct node* const x, const struct node* const y)
{
return x->data > y->data;
}
int main(void)
{
struct node* i = (struct node*) malloc(sizeof(struct node));
i->data = 34;
struct node* j = (struct node*) malloc(sizeof(struct node));
j->data = 89;
if( fun(i, j) ) {
puts("true");
} else {
puts("false");
}
return 0;
}
zub: you might want to explain your changes instead of just posting code.
I find it easier to write code than to explain, because English is not my native language. Moreover, in this particular case, no explanation needed. Compare line by line and you will understand what I have done fixes.
I can further simplify the code with a few useful tricks. Good code is self-evident and self-documenting.
By the way, I forgot to free memory. All the time I forget it, because my own projects use my own allocator, which does not require this.
Code:#include <stdio.h> // puts
#include <stdlib.h> // malloc
#define NEW(x) ((x)malloc(sizeof(struct x)))
typedef struct Node {
int data;
} *Node;
int fun(const Node const x, const Node const y)
{
return x->data > y->data;
}
int main(void)
{
Node i = NEW(Node);
i->data = 34;
Node j = NEW(Node);
j->data = 89;
if( fun(i, j) ) {
puts("true");
} else {
puts("false");
}
free(i);
free(j);
return 0;
}
As an FYI for zub: FAQ > Casting malloc - Cprogramming.com
Ah. Don't worry, the idea is to highlight things that the the help seeker might miss. You don't have to have perfect English to do that.Quote:
Originally Posted by zub
It is not always reasonable to expect a newbie to be able to distinguish between changes that are substantially important, those that are important stylistically, and those that are merely subjective style, just by reading code.Quote:
Originally Posted by zub
Unfortunately, your typedef of struct Node* to be Node makes code involving Node no longer self-evident, and your NEW function-style macro is not self-documenting since it relies on the macro argument being of a struct type. Compare:Quote:
Originally Posted by zub
with:Code:Node i = NEW(Node);
i->data = 34;
In the former, one must already be familiar with the NEW macro and know that for NEW(Node) to work the type struct Node must exist; accessing i->data is surprising since Node does not appear to be a pointer. In the latter, the reader is likely to be familiar with malloc and sizeof, and i->data is not surprising since i is obviously a pointer.Code:struct Node *i = malloc(sizeof(*i));
i->data = 34;
Instead of a NEW macro, I would suggest a function for creating and initialising the node, e.g.,
This can then be called in main with no loss of clarity, e.g.,Code:struct Node *node_init(int data)
{
struct Node *node = malloc(sizeof(*node));
if (node)
{
node->data = data;
}
return node;
}
Code:struct Node *i = node_init(34);
That's fine: gaurav#'s code made the same mistake, and we are certainly not responsible for pointing out all possible mistakes in code posted by someone asking for help.Quote:
Originally Posted by zub
This is true sometimes, but it is very hard to judge in regards to a person reviewing there own code. It is an easy logic trap to fall into, to believe that what you are doing is evident. It relies on the assumption that the person reading the code will have all the same associations as yourself though.Quote:
Originally Posted by zub
For instance, when you read the code it might seem natural to treat Node as a pointer because you have made the association that that's just what Node is. Another person reading or debugging it might have to continually look up the definitions for that variable to make sure they are treating it as the correct type.
Edit: Whoops, wasn't trying to parrot you LaserLight (great minds and all that? No? ahh well :P)