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

Educational "Library" project

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

Problem

The assignment is very open and we have only 4 things it needs to cover:

  • File write and read



  • Use a Struct



  • Export into HTML format



  • use a common sorting algorithm



so I've decided to create a little "Library" with following features:

  • read the input file "Library.dat"



  • List the books on the console



  • Sort the books by name



  • Add new book



  • delete a book by ID



  • check a book in / out



So I'm a beginner at C and would be very happy for a review and some tips on how to improve my project.

Input File:

1;v;121212;121212;1
2;a;121212;121212;0
4;e;121212;121212;1
6;d;121212;121212;1
7;w;121212;121212;0
8;x;121212;121212;1
9;c;121212;121212;1


My Config.h file:

//
//  Library.config
//
//  Created by Me on 03.12.13.
//  Copyright (c) 2013 Me. All rights reserved.
//

// DEFINE

#define MAX_STR_LEN 256
#define MAX_BOOKS 10
#define DATE_LEN 8
#define PATH "/users/xxxxxxx/desktop/Library/Library.dat"


My Code:

```
/*****
* Library Application
*
* Author: Me
* Date: 26.11.2013
* Version: BETA
*
*
*****/

/****
* OSX: fpurge(stdin) insted of fflush(stdin)
*
*
*
*****/

/* TODOs:
*
* Search Function
* Colors (conio.h)?
*
*
*/

/ header files for access to specialized functions/
#include
#include
#include
#include "config.h"

typedef struct {
int ID;
char name[MAX_STR_LEN];
char dateIn[DATE_LEN];
char dateOut[DATE_LEN];
int isIn;
}book;

struct book{
int ID;
char name[MAX_STR_LEN];
char dateIn[DATE_LEN];
char dateOut[DATE_LEN];
int isIn;
};

/ array of my books /
struct book books[MAX_BOOKS];

/ PROTOTYPE OF FUNCTIONS /
int readBookFile();
void printBookList();
void addBook();
void mutateBook(book b); //TODO
int getNextID();
book enterBookData();
void delBook();
int writeBookFile();
void menu();
void checkOut();
void checkIn();
void so

Solution

You don't need the two definitions of book:

typedef struct {..} book ;
struct book {..} ;


What I would normally do is:

typedef struct Book {
...
} Book ;


This allows you to write either:

struct Book b1 ;
Book b1 ;


and in both cases get a Book structure. it also means i can refer to a Book structure within a Book structure:

typedef struct Book {
   ...
   struct Book* prequel ;
} Book ;


I'd change the name of the structure and typedef to Book as most coding standards call for structs and typedefs to have an initial capital letter.

In a lot of places you'll see the typedef and the structure given different names,

typedef struct Book_ {...} Book_T ;


it's up to you to decide what you find easiest to use; I said I like to use the same name for both.

Another thing to look at is the readBookFile function, from reading the code I think you need to look at the following:

You use malloc() to allocate the buffer that the file is read into; however you never free the memmory that is allocated by malloc. It's good practice to make sure that every malloc ends up being freed in every route through the function.

You don't check the return value of malloc; in a program like yours it's very unlikely that the malloc call will fail; however you should check the return value.

Do you even need to use malloc here? If you allocate the buffer on the stack then you don't need to worry about freeing it.

The call to fgets() has the magic number 255 in it, you can and should just use the MAX_STR_LEN #define that you created and used in the malloc. Read the man page for fgets(), it tells you that the function already knows that the last char is resevered for a terminating NULL.

The strtok() function call returns NULL if there are no more tokens, again this is something you should check.

You have virtually identical code repeated several times when you call strtok() for each field, you should consider extracting this out into it's own function.

Code Snippets

typedef struct {..} book ;
struct book {..} ;
typedef struct Book {
...
} Book ;
struct Book b1 ;
Book b1 ;
typedef struct Book {
   ...
   struct Book* prequel ;
} Book ;
typedef struct Book_ {...} Book_T ;

Context

StackExchange Code Review Q#39795, answer score: 4

Revisions (0)

No revisions yet.