我只是想写一个简单的函数来添加一个朋友到UserAccount列表。所有信息都是通过参数提供的。如果用户已经在列表中,我不需要再次添加他,但显示记录说他已经在列表中。我写了这段代码。不确定这是否正确,有什么建议来改进代码吗?这能起作用吗?
int add_friend(UserAccount* user, char Circle, UserAccount* friend)
{
struct UserAccountNode *p;
p = (struct UserAccountNode *) malloc(sizeof(struct UserAccountNode));
while (p != NULL)
if(stricmp(user, p->friend) == 0){
p->next = head; // inserting at the beginning
head = p;
}
else {
printf("%d already exists", friend)
};
}
"不确定这是正确的" -它做你想做的吗?如果是,它可能是正确的;如果不是。嗯…
不,它远没有接近正确。错误的事情列表为取消所有提供了一个很好的理由。这些内容包括(但不限于):
-
你声称你想有条件地添加一个新朋友,但只有当他们还没有在列表中。然而,你做的第一件事就是为一些你甚至还不确定你是否需要的东西分配空间?
-
您的循环没有可能满足的退出条件(除非
malloc()
实际上失败)。p
是唯一的退出条件参数,从来没有赋值给函数中第一行之后的任何,这给它分配了一个您甚至可能不需要的动态分配。也就是你有一个无限循环 -
您将
UserAccount*
传递给stricmp
作为第一个参数,它期望const char*
。 -
你正在比较
p->friend
和user
。但是只是分配了p
所指向的。它的friend
成员是不确定的,即没有定义的内容,但你将未定义的内容发送给stricmp()
以与输入参数user
进行比较。这将调用未定义行为。 -
比较逻辑是反向的。如果字符串不区分大小写 = ,则
stricmp()
返回0;不是不同的。你的逻辑(即使你没有调用未定义的行为,由于前面提到的项目)至少是尝试添加项目到列表中,只有当他们已经存在。 -
通过列表进行两次迭代后,如果奇迹般地if表达式两次求值为true,则创建了一个循环自引用节点,并将您最初拥有的任何列表孤立到深渊中。
-
您正在发送
friend
,UserAccount*
,printf
与"%d"
格式说明符。虽然这个很可能不会使您的程序崩溃,但它仍然是未定义的行为。如果你想用printf()
打印一个指针值,使用"%p"
-
未使用的函数参数确实是您最不需要担心的。它们可能没有被使用,但从好的方面来说,它们同样没有被错误地使用;
user
和friend
不能说的
我不会用"good effort"或"nice try"来表扬你。这段代码甚至不可能编译,而且绝对不可能正确运行。你需要回顾你想要实现的实际算法,并且大量地回顾c中指针和动态内存的使用。
您的代码至少有两个问题:
-
while (p != NULL)
如果
p
不是NULL
,这将是一个无限循环,因为您没有在循环体中更改p
或使用break
之类的语句跳出循环。 -
stricmp(user, p->friend)
你正在使用一个未初始化的变量,这个
p = (struct UserAccountNode *) malloc(sizeof(struct UserAccountNode));
只分配了一个结构体,但是在
while
循环中使用它之前从来没有初始化过它。
也可能出现其他错误,例如从未使用friend
参数,stricmp()
的参数可能是错误的,等等。