C++

Forum

 
2d dynamic array.

ynor123 (6)
Hi I have to create a database of student. User can add and delete student from the DB. I have to do this only with array and not std:vector and only use char and not std:string. I do function that add and work good. But in the function that delete, I try to find why programm is crashing when I want to print the new DB, and I don't find where is the problem and how to correct it.

This is my code :

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
 
#include <iostream>
using namespace std;

char** del(char** database, int length, char* word)
{
	int indexTodel = 0;
	for (int i = 0; i <= length; i++)
	{
		if (strcmp(word, database[i]) == 0)
		{
			indexTodel = i;
		}
	}
	cout << indexTodel;

	char** temp = new char* [length];
	int Indextemp = 0;
	for (int i = 0; i < length; i++)
	{
		if (i != indexTodel)
		{
			temp[Indextemp] = new char[strlen(database[i]) + 1];
			strcpy_s(temp[Indextemp], strlen(database[i]) + 1, database[i]);
			Indextemp++;
			database[i] = nullptr;
		}

		return temp;

	}
}
char** add(char** database, int length, char* word)
{
	char** temp = new char* [length];

	if (database)
	{
		for (int i = 0; i < length - 1; i++)
		{
			temp[i] = new char[strlen(database[i])+1];
			strcpy_s(temp[i], strlen(database[i])+1, database[i]);
			delete database[i];
		}
	}
	temp[length - 1] = new char[strlen(word) + 1]; // PLus one
	strcpy_s(temp[length-1], strlen(word) + 1, word);

	/*for (int j = 0; j < strlen(word); j++)
	{
		temp[length - 1][j] = word[j];
	}
	temp[length - 1][strlen(word)] = '\0';*/

	delete[] database;
	return temp;
}

void display(char** database, int length)
{
	for (int i = 0; i < length-1; i++)
	{
		for (int j = 0; j < database[i][j] !='\0'; j++)
		{
			cout << database[i][j];
		}
		cout << endl;
	}
}

int main()
{
	int lengthDb = 0; int choice = 0;
	char** database = NULL;
	char word[50];

	cout << "1.Add" << endl;
	cout << "2.Delete" << endl;
	cout << "3.Display" << endl;
	cout << "4.Exit" << endl;
	cout << "Enter your choice" << endl;
	while (cin >> choice && choice != 4)
	{
		switch (choice)
		{
		case 1:
			cout << "Enter the name of student to add" << endl;
			cin.ignore(); cin.getline(word, 50);
			lengthDb++;
			database = add(database, lengthDb, word);
			display(database, lengthDb);
			break;
		case 2:
			cout << "Enter the name of student to delete" << endl;
			cin.ignore(); cin.getline(word, 50);
			lengthDb--;
			database = del(database, lengthDb, word);
			display(database, lengthDb);
			break;
		case 3:
			break;
		case 4:
			break;
		}

		cout << "Enter your next choice:" << endl;
	}

	return 0;
}
Last edited on
salem c (3700)
1. You're doing a lot of unnecessary work.
1
2
3
 
temp[i] = new char[strlen(database[i])+1];
strcpy_s(temp[i], strlen(database[i])+1, database[i]);
delete database[i];

is simply
 
 
temp[i] = database[i];


2. delete database[i]; should be delete [] database[i];
You used a new[] to allocate it, so you need a delete[] to deallocate it.

3. return temp;
This is INSIDE the loop, it returns after just one iteration.
You need to make sure you copy all the pointers from database[x] to temp[y] that you want to keep.

4. database[i] = nullptr;
You need to delete[] it, not make it nullptr;
ynor123 (6)
I have done all the modification you have say. But my delete function still don't work
salem c (3700)
Well post the latest code then.
JLBorges (13770)
> But my delete function still don't work

This is a hard problem because of the requirement:
"I have to do this only with array and not std:vector and only use char and not std:string."
I presume that that requirement also implies that smart pointers are ruled out.

It would help if we kept related information together, in s struct; say,
1
2
3
4
5
6
 
struct database
{
    char** data = nullptr ;
    int curr_sz = 0 ; // currently in use
    int reserved_sz = 0 ; // max space available
};


We would find that writing code is easier if we get into the habit of writing smaller functions.
The CoreGuidelines points out:
Keep functions short and simple
Reason Large functions are hard to read, more likely to contain complex code, and more likely to have variables in larger than minimal scopes. Functions with complex control structures are more likely to be long and more likely to hide logical errors.
...
Note “It doesn’t fit on a screen” is often a good practical definition of “far too large.”
One-to-five-line functions should be considered normal.

Note Break large functions up into smaller cohesive and named functions. Small simple functions are easily inlined where the cost of a function call is significant.
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-single


'C++ Coding Standards' by Alexandrescu and Sutter:
Avoid long functions. Avoid deep nesting.

Short is better than long, flat is better than deep:...

Excessive straight-line function length and excessive block nesting depth (e.g., if,
for, while, and try blocks) are twin culprits that make functions more difficult to understand and maintain, and often needlessly so. Each level of nesting adds intellectual overhead when reading code because you need to maintain a mental stack (e.g., enter conditional, enter loop, enter try, enter conditional, ...).



Here is some code for you to look at and play around with. Even if you fully understand this code, you would learn more if you try to write the code (modify your existing code) on your own

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
 
#include <iostream>
#include <algorithm>
#include <cstring>
#include <iomanip>

const int MAX_NAME_SZ = 127 ;

char* get_student( char name[MAX_NAME_SZ+1] ) // get name from user input
{
    std::cout << "enter name (max " << MAX_NAME_SZ << " characters): " ;

    std::cin >> std::ws ; // skip white space (this also skips empty lines)
    std::cin.getline( name, MAX_NAME_SZ+1 ) ;

    return name ;
}

// get name from user input. buffer is allocated with new[], release it with delete[]
char* get_student() { return get_student( new char[MAX_NAME_SZ+1] {} ) ; }

struct database
{
    char** data = nullptr ;
    int curr_sz = 0 ; // currently in use
    int reserved_sz = 0 ; // max space available
};

void destroy( database& db ) noexcept // release all memory
{
    for( int i = 0 ; i < db.curr_sz ; ++i ) delete[] db.data[i] ;
    delete[] db.data ;
    db.data = nullptr ;
    db.curr_sz = db.reserved_sz = 0 ;
}

// TO DO: copy database, assign database

// increase space available to at least new_reserved_sz
void reserve( database& db, int new_reserved_sz ) 
{
    if( db.reserved_sz < new_reserved_sz )
    {
        // allocate at least double the current allocation and a minimum of 16 bytes
        new_reserved_sz = std::max( { db.reserved_sz*2, new_reserved_sz, 16 } ) ;
        char** new_buf = new char*[new_reserved_sz] {} ;

        // copy items to the new buffer
        if(db.data) std::copy( db.data, db.data+db.curr_sz, new_buf ) ;

        // release the old buffer and make the database use the new buffer
        delete[] db.data ;
        db.data = new_buf ;
        db.reserved_sz = new_reserved_sz ;
    }
}

void add_item( database& db )
{
    reserve( db, db.curr_sz+1 ) ; // make sure we have enough space
    db.data[db.curr_sz] = get_student() ;
    ++db.curr_sz ;
}

bool erase( database& db, int pos ) // erase item at position pos
{
    if( pos >= 0 && pos < db.curr_sz )
    {
        delete[] db.data[pos] ; // delete item at pos
        db.data[pos] = nullptr ;

        // move items after pos to the left
        std::move( db.data+pos+1, db.data+db.curr_sz, db.data+pos ) ;
        --db.curr_sz ;

        return true ;
    }

    return false ; // invalid pos
}

bool erase( database& db, const char* text ) // erase first item with matching text (case-sensitive)
{
    for( int i = 0 ; i < db.curr_sz ; ++i )
        if( std::strcmp( db.data[i], text ) == 0 ) return erase( db, i ) ;

    return false ; // no match
}

void print( const database& db )
{
    std::cout << "--------------------------------\n" ;

    for( int i = 0 ; i < db.curr_sz ; ++i )
        std::cout << std::setw(4) << i+1 << ". " << db.data[i] << '\n' ;

    std::cout << "--------------------------------\n" ;
}

int get_int( const char* prompt, int minv, int maxv ) // get input int in the specified range
{
    std::cout << prompt << " [" << minv << ',' << maxv << "]: " ;
    int input ;
    if( std::cin >> input && input >= minv && input <= maxv ) return input ;

    // input failed; clean up and try again
    std::cout << "invalid input. try again\n" ;
    std::cin.clear() ;
    std::cin.ignore( 1'000'000, '\n' ) ;
    return get_int( prompt, minv, maxv ) ;
}

enum option { ADD = 1, DELETE = 2, DISPLAY = 3, QUIT = 4 };

option get_option()
{
    std::cout << '\n' << ADD << ". Add\n" << DELETE << ". Delete\n"
              << DISPLAY << ". Display\n" << QUIT << ". Quit\n" ;

    return option( get_int( "enter choice", ADD, QUIT ) ) ;
}

void process_delete_command( database& db )
{
    std::cout << "delete student with name : " ;
    char name[MAX_NAME_SZ+1] {} ;
    if( erase( db, get_student(name) ) ) std::cout << "erased '" << name << "' from the database\n" ;
    else std::cout << "did not find name '" << name << "' in the database\n" ;
}

int main()
{
    database db ;

    option opt = QUIT ;
    while( ( opt = get_option() ) != QUIT )
    {
        switch(opt)
        {
            case ADD:
                add_item(db) ;
                break ;

            case DELETE:
                process_delete_command(db) ;
                break ;

            case DISPLAY:
                print(db) ;
                break ;

            case QUIT:
                std::cout << "\nbye!\n" ;
        }
    }

    destroy(db) ; // we are done; clean up and quit
}


seeplus (6592)
There are various issues. Consider (not fully tested):

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
 
#include <iostream>
#include <cstring>
using namespace std;

char** del(char** database, int& length, const char* word)
{
	if (length >= 1) {
		char** temp {new char* [length - 1]};
		bool got {};

		for (int i = 0, t = 0; i < length && t <= length - 1; ++i)
			if (!got && strcmp(word, database[i]) == 0) {
				got = true;
				delete[] database[i];
			} else
				temp[t++] = database[i];

		if (got)
			if (--length > 0)
				return temp;
			else {
				delete[] database;
				database = nullptr;
			}
		else
			cout << "Not found\n";

		delete[] temp;
	}

	return database;
}

char** add(char** database, int& length, const char* word)
{
	++length;

	char** temp {new char* [length]};

		for (int i = 0; i < length - 1; ++i)
			temp[i] = database[i];

	temp[length - 1] = new char[strlen(word) + 1];
	strcpy_s(temp[length - 1], strlen(word) + 1, word);

	delete[] database;
	return temp;
}

void display(const char* const *database, int length)
{
	cout << '\n';
	for (int i = 0; i < length; ++i)
		cout << database[i] << '\n';
}

int main()
{
	int lengthDb {};
	int choice {};
	char** database {};
	char word[50] {};

	cout << "1.Add\n";
	cout << "2.Delete\n";
	cout << "3.Display\n";
	cout << "4.Exit\n";
	cout << "Enter your choice";

	while (cin >> choice && choice != 4) {
		cin.ignore();
		switch (choice) {
			case 1:
				cout << "Enter the name of student to add" << endl;
				cin.getline(word, 50);
				database = add(database, lengthDb, word);
				display(database, lengthDb);
				break;

			case 2:
				cout << "Enter the name of student to delete" << endl;
				cin.getline(word, 50);
				database = del(database, lengthDb, word);
				display(database, lengthDb);
				break;

			case 3:
				display(database, lengthDb);
				break;

			case 4:
				break;
		}

		cout << "Enter your next choice:" << endl;
	}
}



1.Add
2.Delete
3.Display
4.Exit
Enter your choice1
Enter the name of student to add
qwerty

qwerty
Enter your next choice:
2
Enter the name of student to delete
qwerty

Enter your next choice:
3

Enter your next choice:
3

Enter your next choice:
1
Enter the name of student to add
qwerty

qwerty
Enter your next choice:
1
Enter the name of student to add
asdfgh

qwerty
asdfgh
Enter your next choice:
1
Enter the name of student to add
zxcvbn

qwerty
asdfgh
zxcvbn
Enter your next choice:
2
Enter the name of student to delete
zxcvbn

qwerty
asdfgh
Enter your next choice:
2
Enter the name of student to delete
asdfgh

qwerty
Enter your next choice:
2
Enter the name of student to delete
qwerty

Enter your next choice:
1
Enter the name of student to add
asdfgh

asdfgh
Enter your next choice:
1
Enter the name of student to add
zxcvbn

asdfgh
zxcvbn
Enter your next choice:
3

asdfgh
zxcvbn
Enter your next choice:
2
Enter the name of student to delete
qwerty
Not found

asdfgh
zxcvbn
Enter your next choice:
2
Enter the name of student to delete
qwerty
Not found

asdfgh
zxcvbn
Enter your next choice:
1
Enter the name of student to add
qwerty

asdfgh
zxcvbn
qwerty
Enter your next choice:
3

asdfgh
zxcvbn
qwerty
Enter your next choice:
2
Enter the name of student to delete
qwerty

asdfgh
zxcvbn
Enter your next choice:
4

Last edited on
jlb (4973)
I have a question about the "add" function.

The first time you call that function where are you allocating memory for "database"?

And for succeeding calls be careful, the pointer to "database" is being passed by value.

JLBorges (13770)
> The first time you call that function where are you allocating memory for "database"?

add_item does this before it does anything else:
reserve( db, db.curr_sz+1 ) ; // make sure we have enough space // *** on line 59

reserve() allocates additional memory if it is required.
reserve() uses a simple doubling strategy to determine the new capacity (new reserved size);
the idea is to make add_item have an amortised constant ( O(1) ) time complexity.
jlb (4973)
add_item does this before it does anything else:

Actually I wasn't questioning your code, but the code of the OP and the code presented by seeplus.

Your code using the structure to encapsulate the array of strings is a better approach. However, IMO, the OP may want to consider using an all out class to fully encapsulate the messy manual allocations/de-allocations to within the class.
JLBorges (13770)
> Your code using the structure to encapsulate the array of strings is a better approach

My main point was about avoiding excessive block nesting depth within a single function;
the code was presented primarily to illustrate that writing simple code is not all that difficult.

For instance, in the code that seeplus presented, presumably as an improvement,
the del() function has this structure:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
 
if
{
    for 
    {
        if
        {
        }

        if 
        {
            if 
            {
            }
            else
            {
            }
        }
        else
        {
        }
    }
}


I was trying to say that this kind of deeply nested code makes software harder than it needs to be;
harder to understand, harder to test, harder to modify and maintain.
seeplus (6592)
I was trying to say that this kind of deeply nested code makes software harder than it needs to be;
harder to understand, harder to test, harder to modify and maintain.


Agree! :) Because of the data structure used, there are various edge cases to be considered. I didn't have time to completely refactor in this case - or even exhaustively test that it worked in all cases! It's quite possible in this type of nested code that a particular case isn't handled/incorrectly handled.
Topic archived. No new replies allowed.