Re: 2D array of structures

svata wrote:


I rather wonder if someone can comment it to help me to learn good
programming habits.

So my code follows:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#define FIRST 7
#define MAX 10

// First of variables have to be declared

Right away, this raft of global variables raises many flags. There are
occasions when globals are the right solution, but for the most part
it's a sign of a lazy or inexperienced programmer.

char answer;
char item_name[MAX];
char *p_item_name;
char string;

"string" is declared as a single character. The name doesn't fit the
type, and is probably an error. I imagine you wanted a character buffer.

int i;
int j;
int str_len;
float amount;
int NEW_SIZE;

Try to reserve all caps for macros.


typedef struct {
// to store strig entered by user

// comments are a feature of c99, although often an available
extension. We try to avoid them here due to line-wrapping.

char *name;
// to be able determine whether it is predefined menu or not, used
when receipt is printed out
int menu;
// to be able determine to which menu group
int group;
// to store price entered by user
float price;
// important as we need to see which index is already used
int used;
} SHOPPING;

Again, don't use all-caps.

SHOPPING *p_shopping;

// Declaring of menu items, necessary for later reuse in array
char *p_menu_items[] = {"Bread", "Butter", "Confectionary", "Fruit",
"Meat", "Milk", "Vegetables", "Other"};

This should be:

const char *p[] = . . .

String literals are not modifiable.


// Declaring choices menu, not being used in any other array
char *p_menu_choices[] = {"Sub-total", "Total", "Quit"};

Same here.


int main() {

int store_data(int); // declaring we want to use function
// now, allocating memory for first 8 arrays, menu is going to be
stored there
p_shopping = malloc(FIRST * sizeof *p_shopping);

FIRST is 7, yet you say you are allocating eight. Which is it? Also,
why dynamic allocation

if ( p_shopping == NULL ) {
printf("Failed to allocate memory, exiting...");
return 1;

Use the standard return values: 0, EXIT_SUCCESS, or EXIT_FAILURE.

}

while (1) { // infinitive loop
// This cannot be part of a menu array, to be printed once only!!
printf("\nEnter your choice: \n");

You prompt for a choice, but I don't see any input statement.

// Putting menu together by looping through 2 arrays
for ( i = 0; i < (FIRST + 1); i++ ) {
// it's time to allocate memory on the fly, as we have
different string size
p_shopping[i].name = malloc((strlen(p_menu_items[i]) + 1) *
sizeof(char));

p_shopping has only 7 structs in the array, with valid indexes of 0-6.
You are attempting to use up to index 7. Boom (if you're lucky).



I'm going to stop here. You have too many problems. You tried to write
the whole program at once, and that seldom works. Even experienced
programmers don't do it that way.

Start over. Start small. Build up the pieces. Get your menu selection
working first. Use functions. At the early stages, those will be stubs,
then get their code expanded.





Brian
.

Quantcast