HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Processing trip route table in C

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
routeprocessingtriptable

Problem

In fact, this is a group assignment for our C programming lesson, but seeking help from others is not considered cheating.

The goal of the following code is to:

  • Sort the data in start.txt by destination, and then by month if two trip routes share one destination.



  • Count how many trip routes each month and destination have.



  • Output the sorted data and the statistics to result.txt.



Since this is my first project, I want its coding style to be reviewed most of all. Also, please let me know if adding register or restricted somewhere can be benefitial, for I want to learn more about the use of modifiers in C. Of course, other optimisations are appreciated too.

```
#include
#include
#include
#include
#include

#define MAX 500

typedef struct info
{
unsigned num;
char dest[80];
unsigned length;
char cat[80];
unsigned month;
unsigned price;
} info;

size_t readIn(info *s);
void overallSort(info *s, size_t itemNum);
size_t groupByDest(info *s, size_t firstItemIndex, size_t itemNum);
void sortByMonth(info *s, size_t lowerBound, size_t upperBound);
void countRouteMonth(info s, size_t itemNum, unsigned numEachMonth);
void countRouteKind(info s, size_t itemNum, unsigned numEachDest);
void output(info s, size_t itemNum, unsigned numEachMonth, unsigned *numEachDest);

int main(void)
{
size_t itemNum;
struct info s[MAX];
unsigned numEachMonth[12];
unsigned numEachDest[MAX] = {0};
itemNum = readIn(s);
overallSort(s, itemNum);
countRouteMonth(s, itemNum, numEachMonth);
countRouteKind(s, itemNum, numEachDest);
output(s, itemNum, numEachMonth, numEachDest);
setlocale(LC_ALL, "");
fwide(stdout, 1);
fputwc(L'\uFEFF', stdout);
wprintf(L"\u4E34\u7801\u6D95\u96F6\uFF0C\u4E0D\u77E5\u6240\u8A00\u3002\n");
wprintf(L"\u5E78\u751A\u81F3\u54C9\uFF0C\u6B4C\u4EE5\u548F\u5FD7\u3002\n");
return EXIT_SUCCESS;
}

size_t readIn(struct info *s)
{
unsigned itemCount = 0;
FILE

Solution

Naming

Structure/variable naming is one of the most effective ways you have of making your code self documenting. It's always worth taking some time to consider your names, this becomes more important the wider the usage of the names. You have names like s, info, num for your most important structure, its members and references to it. These are far from descriptive and are worth reconsidering.

You also have method names like readin, output, what are these methods reading/writing, to where?

Overflow

You readin method loops through a file, reading into a constantly sized structure buffer. If the file contains more lines than the number of buffer elements it will overflow. You should be checking that itemCount < MAX.

Error checking

Your readin method is using sscanf to parse the contents of the file. You're not checking it's return value, what are you expecting it to do if you get passed a line that's not the correct format?

Bracing

This is subjective, but I don't really like this:

for(j = 0; j < 12; j++)
    if(s[i].month == j + 1)
    {


It's error prone if you decide you need to extra behaviour to the for loop and forget to add the braces when you do. I'd prefer to see braces wrapping the for loop...

Code Snippets

for(j = 0; j < 12; j++)
    if(s[i].month == j + 1)
    {

Context

StackExchange Code Review Q#132410, answer score: 3

Revisions (0)

No revisions yet.