C语言实现队列review考虑到的知识点

在stackexchange有一个人提了这么一个问题，自己实现了一个通用队列，然后把代码贴了上来，然后请大家review以下，希望基于以下几方面提一些建议：

• 1，编程风格(My general style）
• 2，是否正确地释放内存以避免内存泄漏
• 3，正确地处理了内存分配失败
• 4，其它任何你能想得到的建议

在我看来，下面的实现其实已经相当完备了，代码正确性，风格，可读性都相当不错，可是下面还有一人竟然提出了12条改进建议，让我觉得相当惊讶，我看了下这个人的简介，coding since 1976，让我觉得一个有着多年经验的程序员和普通程序员之间的差别。
代码如下：

Queue.h

#ifndef QUEUE_H_INCLUDED
#define QUEUE_H_INCLUDED

typedef struct Node
{
  void *data;
  struct Node *next;
}node;

typedef struct QueueList
{
    int sizeOfQueue;
    size_t memSize;
    node *head;
    node *tail;
}Queue;

void queueInit(Queue *q, size_t memSize);
int enqueue(Queue *, const void *);
void dequeue(Queue *, void *);
void queuePeek(Queue *, void *);
void clearQueue(Queue *);
int getQueueSize(Queue *);

#endif /* QUEUE_H_INCLUDED */

Queue.c

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include "Queue.h"

void queueInit(Queue *q, size_t memSize)
{
   q->sizeOfQueue = 0;
   q->memSize = memSize;
   q->head = q->tail = NULL;
}

int enqueue(Queue *q, const void *data)
{
    node *newNode = (node *)malloc(sizeof(node));

    if(newNode == NULL)
    {
        return -1;
    }

    newNode->data = malloc(q->memSize);

    if(newNode->data == NULL)
    {
        free(newNode);
        return -1;
    }

    newNode->next = NULL;

    memcpy(newNode->data, data, q->memSize);

    if(q->sizeOfQueue == 0)
    {
        q->head = q->tail = newNode;
    }
    else
    {
        q->tail->next = newNode;
        q->tail = newNode;
    }

    q->sizeOfQueue++;
    return 0;
}

void dequeue(Queue *q, void *data)
{
    if(q->sizeOfQueue > 0)
    {
        node *temp = q->head;
        memcpy(data, temp->data, q->memSize);

        if(q->sizeOfQueue > 1)
        {
            q->head = q->head->next;
        }
        else
        {
            q->head = NULL;
            q->tail = NULL;
        }

        q->sizeOfQueue--;
        free(temp->data);
        free(temp);
    }
}

void queuePeek(Queue *q, void *data)
{
    if(q->sizeOfQueue > 0)
    {
       node *temp = q->head;
       memcpy(data, temp->data, q->memSize);
    }
}

void clearQueue(Queue *q)
{
  node *temp;

  while(q->sizeOfQueue > 0)
  {
      temp = q->head;
      q->head = temp->next;
      free(temp->data);
      free(temp);
      q->sizeOfQueue--;
  }

  q->head = q->tail = NULL;
}

int getQueueSize(Queue *q)
{
    return q->sizeOfQueue;
}

TestQueue.c

#include <stdio.h>
#include <stdlib.h>
#include "Queue.h"

int main()
{
    int val;
    Queue q;

    queueInit(&q, sizeof(int));

    for(val = 0; val < 10; val++)
    {
        enqueue(&q, &val);
        printf("The value %d has been enqueued.\n", val + 1);
    }

    printf("\n");

    queuePeek(&q, &val);

    printf("The value that is at the front of the queue is %d\n\n", val + 1);

    while(getQueueSize(&q) > 0)
    {
        dequeue(&q, &val);
        printf("%d has been dequeued.\n", val + 1);
    }

    return 0;
}

下面让我看看他提的一些建议：

• 1，Pattern-less function names。对于queue.h里面一些函数的命名， Queue_Init(), Queue_GetSize() 要比 queueInit(), getQueueSize() 更能凸显意义，更具有一致性。
• 2，Comments with # preprocessing may not be portable。注释带了#可能移植性不太好。这个没理解，原程序中似乎也没有这样用注释，哪位读者明白可以评论里面补充下。

// #endif /* QUEUE_H_INCLUDED */
#endif

• 3，QueueList没有必要保存两个head和tail两个指针，只需要保存tail指针，并且tail结点的next指针域指向头结点，这实际上是一个循环链表组成的queue，那么链表走到末端的判断将会是p->next == tail->next。这样的好处是大量使用queue的时候更节省内存，但是逻辑上复杂了些。
• 4 不够明晰为什么队列大小类型使用int表示。int是（有符号）signed类型，可能表示负数，也许可以使用unsigned，在大多数系统中，size_t表示的范围要比int更加宽广，也许使用size_t更加谨慎一些。健壮的代码应该在enqueue函数中检查进队时是否超过队列最大长度。
• 5 好的点：声明memSize为size_t类型，对于malloc()函数的错误检查，有测试用例。另外对于.h的使用有注释说明，对于你的代码来说.h就像是一个公共借口。

关于size_t，int和unsigned int可参考此文https://prateekvjoshi.com/2015/01/03/why-do-we-need-size_t/, size_t,size_t的取值range是目标平台下最大可能的数组尺寸（非负数）,一些平台下size_t的范围小于int的正数范围,又或者大于unsigned int。为了移植性和性能考虑，有些情况下表示大小尽可能使用size_t

• 6 更加健壮的用法在queueInit()函数中检查memSize==0，同样对于malloc()函数应该做以下检查：if(newNode->data == NULL && q->memSize > 0)
• 7 要有些文档化的解释。对于queue.h每个函数声明前面建议由一到两行的注释。
• 8 对于queuePeek(Queue *q, void data)函数，如果q不改变，则应该声明为queuePeek(const Queue *q, void *data)。这是对于q常量特性的自我文档说明，可能对编译器优化也有好处。另外对于函数的使用实现检查也是有好处。
• 9 为了完备性，建议在clearQueue()函数中写上q->memSize = 0 。
• 10 改变#include的包含顺序。把"queue.h"应该放在第一个。这样可以检测到queue.h没有依赖下面的三个<.h>文件。queue.h需要依赖的已经在其本身文件中包含进来。

#include "Queue.h"
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
// #include "Queue.h"

关于头文件依赖顺序说明可以参看：https://zh-google-styleguide.readthedocs.io/en/latest/google-cpp-styleguide/headers/，
1，foo/bar.h.
2，C 系统文件
3，C++系统文件
4，其他库头文件
5，本项目内头文件,
实际上《C++编程思想》对于2,3,4,5建议的顺序可能是反过来的，两者各有利弊。但有一点本文件相关的头文件必须放在第一位，可以防止在本文件（如上面的bar.h）里面少包含了必须的头文件。

• 11 结果声明。对于void dequeue(Queue *q, void *data)函数中拷贝data的操作，没有对结果的声明，应该失败返回fasle，成功返回true。queuePeek()函数也类似。
• 12 为了调试方便，释放内存操作free()之前最好把内存数据置零，这样错误的代码可能在面对原始数据对比为0的指针或者数据更快失败，更容易调试。
• 13 建议：如果非必要，可以不用存储queue的size大小，在需要的时候计算即可。也许可以添加一个检查是否为空队列的函数bool Queue_IsEmpty(const Queue *q)。这样可以去掉size的声明。

另外还有一个回答建议Node结构体的声明可以在.c文件中定义，如果其不是对外公开的数据结构体。

原文网址：

https://codereview.stackexchange.com/questions/141238/implementing-a-generic-queue-in-c