How to put values into array structs?

Question

I'm trying to make account system in C. I'm trying this way to make input to struct arrays.

struct account {                  // Account Structure
int id;
int money;
char *name[30];
};
account * accountarray[50];
int number;


void MakeAccount() {
int id;
int input_money;
char name[50];

printf("--Make Account--\n");
printf("Input ID : ");
scanf("%d", &id);
printf("Insert Money : ");
scanf("%d", &input_money);
printf("Your Name? : ");
scanf("%s", name);

accountarray[number++] = NULL;                // I think there's a problem in this side
accountarray[number++]->id = id;
accountarray[number++]->money = input_money;
*accountarray[number++]->name = name;
}

It stops when i get input the values... I think 4 codes on under has problem.. is there a good way to make it better?

Study pointers and arrays more. You have a struct account type with an array of thirty pointer-to-char (not an array of char, an array of pointer-to-char) as a member, and a global array of fifty pointer-to account. In all of this only one thing actually has any of those pointers pointing to something, and it's a temporary (name). You repeated increment of number between each field assignment looks terribly wrong as well. — WhozCraig
– WhozCraig, Commented Apr 8, 2017 at 3:43
@WhozCraig Ok, Thanks. I think i didn't need pointer in this section. Solved to put values and checked it is ok. Saw other guy used pointer for this, Tried that way and didn't work. I think i really need to study more pointers lol — WiggleJi
– WiggleJi, Commented Apr 8, 2017 at 3:53
Indeed, WhozCraig is right. accountarray[number++] = NULL; followed by accountarray[number++]->id = id;... This mistake (null pointer dereference) should be obvious if you're reading a book. Are you reading a book? — autistic
– autistic, Commented Apr 8, 2017 at 4:38
You can't learn C safely by unguided trial and error; you need a guide (e.g. a book), or you risk relying upon non-portable undefined behaviour such as this. The problem with undefined behaviour is it isn't always guaranteed to produce a crash; in fact, you're lucky in this case as the very definition of undefined behaviour means there are no guarantees. Many problems with undefined behaviour are extremely difficult to debug, such as race conditions, buffer overflows, sequence point abuse... and these can also cause security issues. Stop guessing and start reading! — autistic
– autistic, Commented Apr 8, 2017 at 4:43

user3629249 · Accepted Answer · 2017-04-08 12:52:24Z

here is a version of your code.

it cleanly compiles
it is not executable as there is no main() function
it properly performs error checking
it properly outputs error messages to stderr
it properly declares the array of 'account' data
it makes appropriate use of blank lines to group code blocks and activity blocks
it properly uses a string function to copy the name array
it exits the program when an error occurs
it is consistent in program indenting, for ease of readability
it give the 'magic' numbers meaningful names (and uses those meaningful names throughout the code
it avoids any possibility of overflowing the input buffers
it includes the necessary header files and comments why each header file is being included
it does not consume the final 'newline' sequence entered by the user, however, the call to scanf() for the id will consume that left over byte(s) in stdin as the "%d" input/conversion specifier when entering the next account, will consume leading white space

and now the code

#include <stdio.h>    // printf(), scanf(), perror()
#include <stdlib.h>   // exit(), EXIT_FAILURE
#include <string.h>   // strcpy()

// eliminate 'magic' numbers
#define MAX_STR_LEN 30
#define MAX_ACCTS   50

// define the struct
struct account
{                  // Account Structure
    int id;
    int money;
    char name[ MAX_STR_LEN ];
};

// prototypes
void MakeAccount( void );

// === file global data ===
// declare MAX_ACCTS instances of the struct
struct account accountarray[ MAX_ACCTS ];
// declare a counter
int number = 0;


void MakeAccount()
{
    int id;
    int input_money;
    char name[ MAX_STR_LEN ];

    int scanfStatus;

    printf("--Make Account--\n");

    printf("Input ID : ");
    scanfStatus = scanf("%d", &id);

    if( 1 != scanfStatus )
    { // then scanf failed
        // output error message, including the OS reason for the error to 'stderr'
        perror( "scanf for account id failed" );
        exit( EXIT_FAILURE );
    }

    //implied else, scanf successful

    printf("Insert Money : ");
    scanfStatus = scanf("%d", &input_money);

    if( 1 != scanfStatus )
    { // then scanf failed
        // output error message, including the OS reason for the error to 'stderr'
        perror( "scanf for account money failed" );
        exit( EXIT_FAILURE );
    }

    printf("Your Name? : ");
    scanfStatus = scanf("%29s", name); // note MAX CHARACTERS 1 less than length of input buffer

    if( 1 != scanfStatus )
    { // then scanf failed
        // output error message, including the OS reason for the error to 'stderr'
        perror( "scanf for account name failed" );
        exit( EXIT_FAILURE );
    }

    accountarray[number].id = id;
    accountarray[number].money = input_money;
    strcpy( accountarray[number].name, name);
} // end function: MakeAccount

awesome code, flawless learnt a lot. i should avoid using magic numbers

vim_ · Accepted Answer · 2017-04-08 03:51:53Z

0

I think you probably don't need pointers here because you made a assumption about max size of name as 30 and 50 accounts. So here is the modifications that I think you could make:

 struct account {                  // Account Structure
   int id;
   int money;
   char name[30];  // an array of characters
 };
 struct account accountarray[50]; // Note: its type is "struct account"
 int number;

 void MakeAccount() {
  int id;
  int input_money;
  char name[30];

  printf("--Make Account--\n");
  printf("Input ID : ");
  scanf("%d", &id);
  printf("Insert Money : ");
  scanf("%d", &input_money);
  printf("Your Name? : ");
  scanf("%s", name);

  accountarray[number++].id = id;
  accountarray[number++].money = input_money;
  memcpy(accountarray[number++].name, name, strlen(name)); // Note I used memcpy to copy one array of characters into another
}

answered Apr 8, 2017 at 3:51

vim_

2,20015 silver badges15 bronze badges

3 Comments

user3629249 Over a year ago

actually not a good answer, the returned value from scanf() not being checked, the call to scanf() for name allows the user to overrun the input field, which would result in undefined behavior, missing all the needed #include statements for printf(), scanf(), strlen(), memcpy() Uses 'magic' numbers.

vim_ Over a year ago

@user3629249 :-) you missed a few: not defining a macro for 30. Not defining a macro for 50. And probably I should use unsigned for the id and money. Definitely an unsigned for number. And more importantly void MakeAccount(void) is the correct way to define a function without parameter... But here is a small thing to think about: OP is trying to learn arrays and pointers. :-)

user3629249 Over a year ago

@Arash, I did mention the usage of 'magic' numbers (thats the 30 and 50) Even though the OP is trying to learn arrays and pointers, I believe he should not ignore other items in the code. Best to learn now, rather than develop bad habits that will be difficult to break later on.

ashim888 · Accepted Answer · 2017-04-08 18:36:27Z

0

i think you're trying to make more complex than it should have been, i did it like this

#include <stdio.h>

struct account {                  // Account Structure
    int id;
    int money;
    char name[30];
};

int number;


int main() {
int id,x,input_money;
char name[50];
struct account accountarray[50];

printf("How many Number of accounts do you want to enter? ");
scanf("%d",&x);
printf("--Make Account--\n");
for (int i = 0; i < x; i++)
{

    printf("Input ID : ");
    scanf("%d", &accountarray[i].id);
    printf("Insert Money : ");
    scanf("%d", &accountarray[i].money);
    printf("Your Name? : ");
    scanf("%s", accountarray[i].name);
    printf("\n");
}
printf("****RECORD INSERTED****\n");
for (int i = 0; i < x; i++)
{
    printf("\nID: %d",accountarray[i].id);
    printf("\nNAME: %s",accountarray[i].name);
    printf("\nMONEY: %d",accountarray[i].money);

}

}

you can check https://github.com/ashim888/csit-c for further information and code samples

edited Apr 8, 2017 at 18:36

answered Apr 8, 2017 at 4:01

ashim888

20611 silver badges23 bronze badges

6 Comments

WiggleJi Over a year ago

That's another good way to solve. Thanks! I just changed name part using strcpy

WiggleJi Over a year ago

I really love to.. sorry for my rep points. I'll do it when i get enough :D

ashim888 Over a year ago

@Gomtting no worries :)

user3629249 Over a year ago

regarding this line: void main() { << not portable, will not cleanly compile, except under Visual Studio

user3629249 Over a year ago

Not a good answer, because: uses 'magic' numbers, does not separate code blocks, deviates from the logic and output given by the user, contains several unused variables, does not check that number of accounts desired by the user is <= the number of declared account space, does not check returned value from scanf() to assure the operation was successful. Does not follow the axiom: only one statement per line and (at most) one variable declaration per statement.

|

Collectives™ on Stack Overflow

How to put values into array structs?

3 Answers 3

1 Comment

3 Comments

6 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

1 Comment

3 Comments

6 Comments

Your Answer

Sign up or log in

Post as a guest

Related