Problems with STL list, lists for a class

[juanwalker]

Greetings!!

I'm trying to write and access a list of persons where


This code tries to write one person and print it again. The problems is that I can wirte correctly the person but when i try to read it appears radomnly data on memory. Can anyone tell me please what is happenig here. The program is very simple

I'm working on Dev-C++ 4.9

Here is the code (Doesn't have syntax problems)


-----------------8<---------------------------

#include <windows.h>
#include <stdio.h>
#include <conio.h>
#include <math.h>
#include <list>

#define amount 15

using namespace std;

class Person
{

	public:
	long  id;
	float* codes;
	
	  Person(long id,float codes[])
		{
			this->id=id;
			this->codes=codes;	
		
		}		
		
};

class Codes
{



	public:
	
     Codes()
	{
       	list<Person> Persons;
                                                     
		this->loadNumbers(Persons);
		
        Person p1 = Persons.front();
        for (int i=0; i< amount ;i++
        printf("\n Person code[%d] = %f",i,p1.codes[i]);		    
        printf("\n\n"); 
		       	

	}
	
	private:	

	
	
	void loadNumbers(list<Person>& Persons)
	{
		float codes[amount];
		long id;
		
	

			for(int i=0; i <amount  ;i++)	
			{
				codes[i]=i/3+pow(i,2);
				
            }   
            
            Person p1(id,codes) ;	
			Persons.push_back(p1);
			
            Person p2   = Persons.front();
            for (int i=0; i< amount;i++)
            {
               printf("\n Person code[%d] = %f",i,p2.codes[i]);		   
            }    
            printf("\n\n"); 	
			       
	
	
	}
	
};
int  main(int argc, char *argv[])
{
 	Codes* codes = new Codes();
    getch();
} 

-----------------8<---------------------------

[kempofighter]

This code is not easily readable. Please reformat it within your text editor with proper spacing and tabbing. Then repaste it, highlight it, and press the # symbol to the right of the edit box.

I can tell you what one problem is. Your person constructor is copying a pointer to an array but not the entire array. You are mistakenly thinking that you can simply copy the array as though that is a deep copy. The result is that the person contructor keeps a pointer that was allocated on the stack of the codes::loadNumbers function. This will eventually result in undefined behavior. One way to solve that is to make the float array within the Person class a std::vector<float> and use the assign member function to copy the array passed to the constructure.
http://cplusplus.com/reference/stl/vector/assign/

[juanwalker]

Sorry, now it's readable

[juanwalker]

Many thank for your reply now I understand the problem. Actually the var float codes[] has a static size limited by the constant amount (15 in this case). If I try to do this modification

#define amount 15
class Person
{

	public:
	long  id;
	float codes[amount];
	
	  Person(long id,float codes[amount])
		{
			this->id=id;
			this->codes=codes;	
		
		}		
		
};

But this message is appearing :

incompatible types in assignment of `float*' to `float[15]'

i.e there's any function like strcpy for numeric vals??

[juanwalker]

Thank you for help me, now it's working!

#include <math.h>
#include <list>

#define amount 15

using namespace std;

class Person
{

	public:
	long  id;
	float codes[amount];
	
	  Person(long id,float codes[])
		{
			this->id=id;
			for (int i=0;i < amount ; i++)
			    this->codes[i]=codes[i];
		
		}		
		
};

class Codes
{



	public:
	
     Codes()
	{
       	list<Person> Persons;
                                                     
		this->loadNumbers(Persons);
		
        Person p1 = Persons.front();
        for (int i=0; i< amount ;i++)
  		    printf("\n Person code[%d] = %f",i,p1.codes[i]);		    
        printf("\n\n"); 
		       	

	}
	
	private:	

	
	
	void loadNumbers(list<Person>& Persons)
	{
		float codes[amount];
		long id;
		
	

			for(int i=0; i <amount  ;i++)	
			{
				codes[i]=i/3+pow(i,2);
				
            }   
            
            Person p1(id,codes) ;	
			Persons.push_back(p1);
			
            Person p2   = Persons.front();
            for (int i=0; i< amount;i++)
            {
                printf("\n Person code[%d] = %f",i,p2.codes[i]);		   
            }    
            printf("\n\n"); 	
            for (int i=0; i< amount;i++)
            {
                printf("\n Person code[%d] = %f",i,p2.codes[i]);		   
            }    
            printf("\n\n"); 	
            printf("\n\n"); 	
            for (int i=0; i< amount;i++)
            {
                printf("\n Person code[%d] = %f",i,p2.codes[i]);		   
            }    
            printf("\n\n"); 	
            printf("\n\n"); 	
            for (int i=0; i< amount;i++)
            {
                printf("\n Person code[%d] = %f",i,p2.codes[i]);		   
            }    
            printf("\n\n"); 	
            
    
	
	        
	}
	
};
int  main(int argc, char *argv[])
{
 	Codes* codes = new Codes();
    getch();
} 

[kempofighter]

Good job. Thanks for updating the post with tags. Much better! Another way is with std::copy which really just encapsulates the loop for you. It is a good habit to use std::copy because it is more object oriented and works with all kinds of containers and simple arrays. It also reduces typing on your part. There are many ways of solving the problem and what you did is fine as well!

    // better to make amount a constant instead of a macro substitution.
    const unsigned int amount(15);

    // Example using std::copy
    Person(long id,float codes[])
    {
        this->id=id;
        std::copy(codes, codes + amount, this->codes);
        /*for(int i=0;i < amount ; i++)
            this->codes[i]=codes[i];*/

    }

[juanwalker]

Thank you so much for reply me amigo. I've improved my code now.

have a great day!