Array Dynamic resize in heapO(lg n) algorithm for a fibonacci number with c++ templateNecklace counting problem-with consecutive prime constraintCounting words, letters, average word length, and letter frequencyC++ object poolUse of containers for storing StudentsC# to C++ function call that fills and auto resize array if size is not enoughGeneric Template implementation of merge sort in C++Binary searching the turning point of a functionSymbolic algebra using a generic smart pointer classC++11 min-heap with configurable arity

Why doesn't WotC use established keywords on all new cards?

Purpose of のは in this sentence?

Has a commercial or military jet bi-plane ever been manufactured?

How do I make a function that generates nth natural number that isn't a perfect square?

How to apply differences on part of a list and keep the rest

How can I close a gap between my fence and my neighbor's that's on his side of the property line?

BOOM! Perfect Clear for Mr. T

Hyperlink on red background

As matter approaches a black hole, does it speed up?

Expressing 'our' for objects belonging to our apartment

Independent, post-Brexit Scotland - would there be a hard border with England?

Can a nothic's Weird Insight action discover secrets about a player character that the character doesn't know about themselves?

Are there any Final Fantasy Spirits in Super Smash Bros Ultimate?

What is the difference between 'unconcealed' and 'revealed'?

What is the name of this hexagon/pentagon polyhedron?

I have a unique character that I'm having a problem writing. He's a virus!

How long would it take for people to notice a mass disappearance?

How can I get a job without pushing my family's income into a higher tax bracket?

What are the differences between credential stuffing and password spraying?

Can there be a single technologically advanced nation, in a continent full of non-technologically advanced nations?

Send iMessage from Firefox

How did Kirk identify Gorgan in "And the Children Shall Lead"?

What property of a BJT transistor makes it an amplifier?

Would glacier 'trees' be plausible?



Array Dynamic resize in heap


O(lg n) algorithm for a fibonacci number with c++ templateNecklace counting problem-with consecutive prime constraintCounting words, letters, average word length, and letter frequencyC++ object poolUse of containers for storing StudentsC# to C++ function call that fills and auto resize array if size is not enoughGeneric Template implementation of merge sort in C++Binary searching the turning point of a functionSymbolic algebra using a generic smart pointer classC++11 min-heap with configurable arity






.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty margin-bottom:0;








6












$begingroup$


I have answered a Question in Stackoverflow link.




a) Create a function called resize that can be used to increase the
size of integer arrays dynamically. The function takes three
parameters. The first parameter is the original array, the second
parameter is the size of this array, and the third parameter is the
size of the larger array to be created by this function. Make sure
that you allocate memory from the heap inside this function. After
allocating memory for the second array the function must copy the
elements from the first array into the larger array. Finally, the
function must return a pointer to the new array.



b. In main, allocate an array on the heap that is just large enough to
store the integers 5, 7, 3, and 1.



c. Resize the array to store 10 integers by calling the resize
function created in step a. Remove the old (smaller) array from the
heap. Add the numbers 4, 2, and 8 to the end of the new array.



d. Write a sort function that sorts any integer array in increasing
order.



e. Use the sort function to sort the array of numbers in c above.
Display the sorted numbers.




Is there a Dangling pointer issue.



#include <array>
#include <iostream>

void swap(int *xp, int *yp)

int temp = *xp;
*xp = *yp;
*yp = temp;


//Bubble Sort
bool sort(int arr[], int size)

for( int i = 0; i< size -1; i++)

for( int j = 0; j < size - i -1; j++)

//descending order
if(arr[j]<arr[j+1])

swap(&arr[j], &arr[j+1]);



return true;


void Print(int Array[], int nSize)

for( int i = 0; i < nSize; i++)

std::cout<<" "<<Array[i];

std::cout<<"n";


void Resize( int *&Array, const int& nSizeOld, const int& nSize )

int * newArray = new int[nSize];

//Copy Elements of the Array
for(int i = 0; i< nSize; i++)

newArray[i] = Array[i];


delete[] Array;

//Assign ptr of Prev to new Array
Array = newArray;


int _tmain(int argc, _TCHAR* argv[])

const int kNewSize = 10, kSize = 5;
int *pMyArray = new int[kSize];

//Set Values
for( int i = 0; i< kSize; ++i )

pMyArray[i] = i * 5;


Resize( pMyArray, kSize, kNewSize );

//Set Values
for( int i = kSize; i< kNewSize; ++i )

pMyArray[i] = i * 10;


Print(pMyArray, kNewSize);

sort(pMyArray, kNewSize);

Print(pMyArray, kNewSize);

if( pMyArray!=NULL )

delete[] pMyArray;


return 0;










share|improve this question











$endgroup$







  • 1




    $begingroup$
    As Roland Illig points out below, in C++ there's no need to implement your own low level memory management for this when you can just use std::vector instead. That said, if you do need/want to implement your own low level array resizing, you should be using std::realloc(). The reference page I linked to even provides some (to my inexpert eye) decent example code.
    $endgroup$
    – Ilmari Karonen
    Apr 24 at 11:56











  • $begingroup$
    @IlmariKaronen In fact, you pretty much shouldn't be using realloc in C++ code ...
    $endgroup$
    – L. F.
    Apr 27 at 1:40

















6












$begingroup$


I have answered a Question in Stackoverflow link.




a) Create a function called resize that can be used to increase the
size of integer arrays dynamically. The function takes three
parameters. The first parameter is the original array, the second
parameter is the size of this array, and the third parameter is the
size of the larger array to be created by this function. Make sure
that you allocate memory from the heap inside this function. After
allocating memory for the second array the function must copy the
elements from the first array into the larger array. Finally, the
function must return a pointer to the new array.



b. In main, allocate an array on the heap that is just large enough to
store the integers 5, 7, 3, and 1.



c. Resize the array to store 10 integers by calling the resize
function created in step a. Remove the old (smaller) array from the
heap. Add the numbers 4, 2, and 8 to the end of the new array.



d. Write a sort function that sorts any integer array in increasing
order.



e. Use the sort function to sort the array of numbers in c above.
Display the sorted numbers.




Is there a Dangling pointer issue.



#include <array>
#include <iostream>

void swap(int *xp, int *yp)

int temp = *xp;
*xp = *yp;
*yp = temp;


//Bubble Sort
bool sort(int arr[], int size)

for( int i = 0; i< size -1; i++)

for( int j = 0; j < size - i -1; j++)

//descending order
if(arr[j]<arr[j+1])

swap(&arr[j], &arr[j+1]);



return true;


void Print(int Array[], int nSize)

for( int i = 0; i < nSize; i++)

std::cout<<" "<<Array[i];

std::cout<<"n";


void Resize( int *&Array, const int& nSizeOld, const int& nSize )

int * newArray = new int[nSize];

//Copy Elements of the Array
for(int i = 0; i< nSize; i++)

newArray[i] = Array[i];


delete[] Array;

//Assign ptr of Prev to new Array
Array = newArray;


int _tmain(int argc, _TCHAR* argv[])

const int kNewSize = 10, kSize = 5;
int *pMyArray = new int[kSize];

//Set Values
for( int i = 0; i< kSize; ++i )

pMyArray[i] = i * 5;


Resize( pMyArray, kSize, kNewSize );

//Set Values
for( int i = kSize; i< kNewSize; ++i )

pMyArray[i] = i * 10;


Print(pMyArray, kNewSize);

sort(pMyArray, kNewSize);

Print(pMyArray, kNewSize);

if( pMyArray!=NULL )

delete[] pMyArray;


return 0;










share|improve this question











$endgroup$







  • 1




    $begingroup$
    As Roland Illig points out below, in C++ there's no need to implement your own low level memory management for this when you can just use std::vector instead. That said, if you do need/want to implement your own low level array resizing, you should be using std::realloc(). The reference page I linked to even provides some (to my inexpert eye) decent example code.
    $endgroup$
    – Ilmari Karonen
    Apr 24 at 11:56











  • $begingroup$
    @IlmariKaronen In fact, you pretty much shouldn't be using realloc in C++ code ...
    $endgroup$
    – L. F.
    Apr 27 at 1:40













6












6








6


1



$begingroup$


I have answered a Question in Stackoverflow link.




a) Create a function called resize that can be used to increase the
size of integer arrays dynamically. The function takes three
parameters. The first parameter is the original array, the second
parameter is the size of this array, and the third parameter is the
size of the larger array to be created by this function. Make sure
that you allocate memory from the heap inside this function. After
allocating memory for the second array the function must copy the
elements from the first array into the larger array. Finally, the
function must return a pointer to the new array.



b. In main, allocate an array on the heap that is just large enough to
store the integers 5, 7, 3, and 1.



c. Resize the array to store 10 integers by calling the resize
function created in step a. Remove the old (smaller) array from the
heap. Add the numbers 4, 2, and 8 to the end of the new array.



d. Write a sort function that sorts any integer array in increasing
order.



e. Use the sort function to sort the array of numbers in c above.
Display the sorted numbers.




Is there a Dangling pointer issue.



#include <array>
#include <iostream>

void swap(int *xp, int *yp)

int temp = *xp;
*xp = *yp;
*yp = temp;


//Bubble Sort
bool sort(int arr[], int size)

for( int i = 0; i< size -1; i++)

for( int j = 0; j < size - i -1; j++)

//descending order
if(arr[j]<arr[j+1])

swap(&arr[j], &arr[j+1]);



return true;


void Print(int Array[], int nSize)

for( int i = 0; i < nSize; i++)

std::cout<<" "<<Array[i];

std::cout<<"n";


void Resize( int *&Array, const int& nSizeOld, const int& nSize )

int * newArray = new int[nSize];

//Copy Elements of the Array
for(int i = 0; i< nSize; i++)

newArray[i] = Array[i];


delete[] Array;

//Assign ptr of Prev to new Array
Array = newArray;


int _tmain(int argc, _TCHAR* argv[])

const int kNewSize = 10, kSize = 5;
int *pMyArray = new int[kSize];

//Set Values
for( int i = 0; i< kSize; ++i )

pMyArray[i] = i * 5;


Resize( pMyArray, kSize, kNewSize );

//Set Values
for( int i = kSize; i< kNewSize; ++i )

pMyArray[i] = i * 10;


Print(pMyArray, kNewSize);

sort(pMyArray, kNewSize);

Print(pMyArray, kNewSize);

if( pMyArray!=NULL )

delete[] pMyArray;


return 0;










share|improve this question











$endgroup$




I have answered a Question in Stackoverflow link.




a) Create a function called resize that can be used to increase the
size of integer arrays dynamically. The function takes three
parameters. The first parameter is the original array, the second
parameter is the size of this array, and the third parameter is the
size of the larger array to be created by this function. Make sure
that you allocate memory from the heap inside this function. After
allocating memory for the second array the function must copy the
elements from the first array into the larger array. Finally, the
function must return a pointer to the new array.



b. In main, allocate an array on the heap that is just large enough to
store the integers 5, 7, 3, and 1.



c. Resize the array to store 10 integers by calling the resize
function created in step a. Remove the old (smaller) array from the
heap. Add the numbers 4, 2, and 8 to the end of the new array.



d. Write a sort function that sorts any integer array in increasing
order.



e. Use the sort function to sort the array of numbers in c above.
Display the sorted numbers.




Is there a Dangling pointer issue.



#include <array>
#include <iostream>

void swap(int *xp, int *yp)

int temp = *xp;
*xp = *yp;
*yp = temp;


//Bubble Sort
bool sort(int arr[], int size)

for( int i = 0; i< size -1; i++)

for( int j = 0; j < size - i -1; j++)

//descending order
if(arr[j]<arr[j+1])

swap(&arr[j], &arr[j+1]);



return true;


void Print(int Array[], int nSize)

for( int i = 0; i < nSize; i++)

std::cout<<" "<<Array[i];

std::cout<<"n";


void Resize( int *&Array, const int& nSizeOld, const int& nSize )

int * newArray = new int[nSize];

//Copy Elements of the Array
for(int i = 0; i< nSize; i++)

newArray[i] = Array[i];


delete[] Array;

//Assign ptr of Prev to new Array
Array = newArray;


int _tmain(int argc, _TCHAR* argv[])

const int kNewSize = 10, kSize = 5;
int *pMyArray = new int[kSize];

//Set Values
for( int i = 0; i< kSize; ++i )

pMyArray[i] = i * 5;


Resize( pMyArray, kSize, kNewSize );

//Set Values
for( int i = kSize; i< kNewSize; ++i )

pMyArray[i] = i * 10;


Print(pMyArray, kNewSize);

sort(pMyArray, kNewSize);

Print(pMyArray, kNewSize);

if( pMyArray!=NULL )

delete[] pMyArray;


return 0;







c++ c++11 pointers






share|improve this question















share|improve this question













share|improve this question




share|improve this question








edited Apr 24 at 7:08









200_success

132k20159424




132k20159424










asked Apr 24 at 5:33









f1r361rdf1r361rd

365




365







  • 1




    $begingroup$
    As Roland Illig points out below, in C++ there's no need to implement your own low level memory management for this when you can just use std::vector instead. That said, if you do need/want to implement your own low level array resizing, you should be using std::realloc(). The reference page I linked to even provides some (to my inexpert eye) decent example code.
    $endgroup$
    – Ilmari Karonen
    Apr 24 at 11:56











  • $begingroup$
    @IlmariKaronen In fact, you pretty much shouldn't be using realloc in C++ code ...
    $endgroup$
    – L. F.
    Apr 27 at 1:40












  • 1




    $begingroup$
    As Roland Illig points out below, in C++ there's no need to implement your own low level memory management for this when you can just use std::vector instead. That said, if you do need/want to implement your own low level array resizing, you should be using std::realloc(). The reference page I linked to even provides some (to my inexpert eye) decent example code.
    $endgroup$
    – Ilmari Karonen
    Apr 24 at 11:56











  • $begingroup$
    @IlmariKaronen In fact, you pretty much shouldn't be using realloc in C++ code ...
    $endgroup$
    – L. F.
    Apr 27 at 1:40







1




1




$begingroup$
As Roland Illig points out below, in C++ there's no need to implement your own low level memory management for this when you can just use std::vector instead. That said, if you do need/want to implement your own low level array resizing, you should be using std::realloc(). The reference page I linked to even provides some (to my inexpert eye) decent example code.
$endgroup$
– Ilmari Karonen
Apr 24 at 11:56





$begingroup$
As Roland Illig points out below, in C++ there's no need to implement your own low level memory management for this when you can just use std::vector instead. That said, if you do need/want to implement your own low level array resizing, you should be using std::realloc(). The reference page I linked to even provides some (to my inexpert eye) decent example code.
$endgroup$
– Ilmari Karonen
Apr 24 at 11:56













$begingroup$
@IlmariKaronen In fact, you pretty much shouldn't be using realloc in C++ code ...
$endgroup$
– L. F.
Apr 27 at 1:40




$begingroup$
@IlmariKaronen In fact, you pretty much shouldn't be using realloc in C++ code ...
$endgroup$
– L. F.
Apr 27 at 1:40










3 Answers
3






active

oldest

votes


















18












$begingroup$

If you had tagged this code as C, it would have been acceptable. Since you tagged it as C++, it's horrible.



Instead of writing your own swap function, there's already std::swap in <algorithm>.



Instead of writing bubble sort yourself, just use std::sort, also from <algorithm>.



Instead of using arrays and resizing them yourself, just use std::vector<int>, from <vector>.



After applying these transformations, you cannot have a dangling pointer anymore since your code is completely pointer-free.



As part of an exercise for learning the basic operations on memory management, it's ok to write code like this, but never ever use such code in production. In production the code should look like this:



#include <algorithm>
#include <iostream>
#include <vector>

void Print(const std::vector<int> &nums)

for(int num : nums)

std::cout << " " << num;

std::cout << "n";


int main()

std::vector<int> nums 5, 7, 3, 1 ;

// There's probably a more elegant way to add the elements to the vector.
nums.push_back(4);
nums.push_back(2);
nums.push_back(8);

std::sort(nums.begin(), nums.end());

Print(nums);



By the way, your original code doesn't have any dangling pointer as well. Well done.



You don't need the != NULL check before the delete[] since that pointer cannot be null. In modern C++ (since C++11 I think) you would also write nullptr instead of NULL. The reason is that historically NULL had not been guaranteed to be of pointer type.



Have a look at https://en.cppreference.com/w/cpp/algorithm for more algorithms that you shouldn't implement yourself in C++.



I would have liked to write the push_back block in a shorter way, as well as the Print function. I'm sure there's a more elegant way, I just don't know it.






share|improve this answer











$endgroup$












  • $begingroup$
    I would just stream char-literals instead of length-one string-literals. And cppreference.com is inho a better reference.
    $endgroup$
    – Deduplicator
    Apr 24 at 11:45











  • $begingroup$
    @Deduplicator I changed the link, thanks for the hint. Regarding the single characters: shouldn't the compiler produce exactly the same code for these two variants by detecting the length 1 string? I thought this was possible in C++. I like code that is as uniform as possible, and using string literals for anything string-like seems simple and nice to me.
    $endgroup$
    – Roland Illig
    Apr 24 at 18:33











  • $begingroup$
    The compiler could in theory inline enough layers to get the same code in the end using whole-program-analysis, necessary due to virtual functions called. I doubt it though.
    $endgroup$
    – Deduplicator
    Apr 24 at 19:15










  • $begingroup$
    "You don't need the != NULL check before the delete[] since that pointer cannot be null" The behaviour is identical anyway, as the delete and delete[] operators are both defined to be null safe (in which case they do nothing).
    $endgroup$
    – Max Barraclough
    Apr 25 at 21:57










  • $begingroup$
    Thanks for the Review
    $endgroup$
    – f1r361rd
    2 days ago


















4












$begingroup$

The code is obviously wrong: your compiler should have warmed you that Resize() never uses its nSizeOld parameter.






share|improve this answer









$endgroup$








  • 1




    $begingroup$
    That's a funny concept of "wrong"... Anyway, it was probably a typo, since it actually is needed; the question was edited accordingly. This answer is superfluous now.
    $endgroup$
    – Anakhand
    Apr 24 at 6:58







  • 2




    $begingroup$
    Please see What to do when someone answers. I have rolled back Rev 2 → 1.
    $endgroup$
    – 200_success
    Apr 24 at 7:09






  • 4




    $begingroup$
    If we follow those rules literally, to correct [what seems to me] a simple typo, OP should ask a completely new question with the fix. Seems kind of ridiculous for such a trivial matter... If every question followed that rule verbatim this site would me a mess. I'm not saying your comment isn't correct or helpful, but it should be precisely that, a comment, so that OP can fix the [irrelevant] issue and move on.
    $endgroup$
    – Anakhand
    Apr 24 at 8:11







  • 2




    $begingroup$
    While your answer is correct and you made a good observation I'd contend that this type of error (a trivial bug) was not what the OP wanted to ask (it would, after all, be OT), so that it does not warrant an answer. I'd have written a comment and waited for the correction. You should imo reinstate the corrected version of the question and delete (or preface as obsolete) your answer.
    $endgroup$
    – Peter A. Schneider
    Apr 24 at 17:10







  • 1




    $begingroup$
    @PeterA.Schneider As per the rules in the help center, all aspects of the code are open to critique. This out-of-bounds memory access bug (which may or may not have been apparent to the author) is absolutely fair game for a Code Review answer.
    $endgroup$
    – 200_success
    Apr 24 at 17:17


















2












$begingroup$

  1. Your code is too low-level. It expresses implementation details instead of intent. That's why your code looks like "C with couts instead of printf and new/delete instead of malloc/free" instead of C++.



  2. Roland Illig has already told you that you should use std::swap instead of building a new one from scratch. You should use existing libraries, especially the standard library, whenever possible.



    That said, your own implementation of swap is also questionable. This is C++, not C. We have references. Using pointers makes the code less readable, and puts burden on the user of the function. So you should change it to:




    void swap(int& x, int& y)

    int temp = x;
    x = y;
    y = temp;



    And the calls to it can be changed from swap(&foo, &bar) to swap(foo, bar). Still, std::swap is preferable.



  3. Again, Roland Illig has already told you that you should use the std::sort instead of building a new bubble sort from scratch. std::sort typically uses quicksort, which has $O(n log n)$ time complexity; whereas bubble sort has $O(n^2)$ time complexity. It should be obvious that std::sort is much more efficient.



  4. Your parameter lists are so C-ish. (pointer, size) parameter pairs are everywhere. They are error-prone. Consider using spans. (Spans are currently not available in the standard library; consider using the one from GSL)



    You even have parameter lists like (int*& Array, const int& nSizeOld, const int& nSize). Don't pass by const reference for builtin types. Just pass by value, as in int nSizeOld, int nSize. And letting a pointer denote an array with sizes littered everywhere holds a great welcome party for errors.




  5. Don't use _tmain and _TCHAR. They are not portable. (Strictly speaking, they are not proper C++.) You should write in ISO standard C++. Use main and char instead.




    // Correct prototype of the main function
    int main(int argc, char* argv[])

    // ...



  6. Don't make such liberal use of "naked" news and deletes. Explicit calls to news and deletes are very error prone. std::vectors should be preferred from the beginning.


  7. You have four for loops in total. The first three use i++, whereas the last one uses ++i. Please consistently use ++i.


As a conclusion: you should refactor your code to express intent.






share|improve this answer









$endgroup$












  • $begingroup$
    Thanks for the Review.
    $endgroup$
    – f1r361rd
    2 days ago











Your Answer






StackExchange.ifUsing("editor", function ()
StackExchange.using("externalEditor", function ()
StackExchange.using("snippets", function ()
StackExchange.snippets.init();
);
);
, "code-snippets");

StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "196"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);

StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);

else
createEditor();

);

function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader:
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
,
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);



);













draft saved

draft discarded


















StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f218994%2farray-dynamic-resize-in-heap%23new-answer', 'question_page');

);

Post as a guest















Required, but never shown

























3 Answers
3






active

oldest

votes








3 Answers
3






active

oldest

votes









active

oldest

votes






active

oldest

votes









18












$begingroup$

If you had tagged this code as C, it would have been acceptable. Since you tagged it as C++, it's horrible.



Instead of writing your own swap function, there's already std::swap in <algorithm>.



Instead of writing bubble sort yourself, just use std::sort, also from <algorithm>.



Instead of using arrays and resizing them yourself, just use std::vector<int>, from <vector>.



After applying these transformations, you cannot have a dangling pointer anymore since your code is completely pointer-free.



As part of an exercise for learning the basic operations on memory management, it's ok to write code like this, but never ever use such code in production. In production the code should look like this:



#include <algorithm>
#include <iostream>
#include <vector>

void Print(const std::vector<int> &nums)

for(int num : nums)

std::cout << " " << num;

std::cout << "n";


int main()

std::vector<int> nums 5, 7, 3, 1 ;

// There's probably a more elegant way to add the elements to the vector.
nums.push_back(4);
nums.push_back(2);
nums.push_back(8);

std::sort(nums.begin(), nums.end());

Print(nums);



By the way, your original code doesn't have any dangling pointer as well. Well done.



You don't need the != NULL check before the delete[] since that pointer cannot be null. In modern C++ (since C++11 I think) you would also write nullptr instead of NULL. The reason is that historically NULL had not been guaranteed to be of pointer type.



Have a look at https://en.cppreference.com/w/cpp/algorithm for more algorithms that you shouldn't implement yourself in C++.



I would have liked to write the push_back block in a shorter way, as well as the Print function. I'm sure there's a more elegant way, I just don't know it.






share|improve this answer











$endgroup$












  • $begingroup$
    I would just stream char-literals instead of length-one string-literals. And cppreference.com is inho a better reference.
    $endgroup$
    – Deduplicator
    Apr 24 at 11:45











  • $begingroup$
    @Deduplicator I changed the link, thanks for the hint. Regarding the single characters: shouldn't the compiler produce exactly the same code for these two variants by detecting the length 1 string? I thought this was possible in C++. I like code that is as uniform as possible, and using string literals for anything string-like seems simple and nice to me.
    $endgroup$
    – Roland Illig
    Apr 24 at 18:33











  • $begingroup$
    The compiler could in theory inline enough layers to get the same code in the end using whole-program-analysis, necessary due to virtual functions called. I doubt it though.
    $endgroup$
    – Deduplicator
    Apr 24 at 19:15










  • $begingroup$
    "You don't need the != NULL check before the delete[] since that pointer cannot be null" The behaviour is identical anyway, as the delete and delete[] operators are both defined to be null safe (in which case they do nothing).
    $endgroup$
    – Max Barraclough
    Apr 25 at 21:57










  • $begingroup$
    Thanks for the Review
    $endgroup$
    – f1r361rd
    2 days ago















18












$begingroup$

If you had tagged this code as C, it would have been acceptable. Since you tagged it as C++, it's horrible.



Instead of writing your own swap function, there's already std::swap in <algorithm>.



Instead of writing bubble sort yourself, just use std::sort, also from <algorithm>.



Instead of using arrays and resizing them yourself, just use std::vector<int>, from <vector>.



After applying these transformations, you cannot have a dangling pointer anymore since your code is completely pointer-free.



As part of an exercise for learning the basic operations on memory management, it's ok to write code like this, but never ever use such code in production. In production the code should look like this:



#include <algorithm>
#include <iostream>
#include <vector>

void Print(const std::vector<int> &nums)

for(int num : nums)

std::cout << " " << num;

std::cout << "n";


int main()

std::vector<int> nums 5, 7, 3, 1 ;

// There's probably a more elegant way to add the elements to the vector.
nums.push_back(4);
nums.push_back(2);
nums.push_back(8);

std::sort(nums.begin(), nums.end());

Print(nums);



By the way, your original code doesn't have any dangling pointer as well. Well done.



You don't need the != NULL check before the delete[] since that pointer cannot be null. In modern C++ (since C++11 I think) you would also write nullptr instead of NULL. The reason is that historically NULL had not been guaranteed to be of pointer type.



Have a look at https://en.cppreference.com/w/cpp/algorithm for more algorithms that you shouldn't implement yourself in C++.



I would have liked to write the push_back block in a shorter way, as well as the Print function. I'm sure there's a more elegant way, I just don't know it.






share|improve this answer











$endgroup$












  • $begingroup$
    I would just stream char-literals instead of length-one string-literals. And cppreference.com is inho a better reference.
    $endgroup$
    – Deduplicator
    Apr 24 at 11:45











  • $begingroup$
    @Deduplicator I changed the link, thanks for the hint. Regarding the single characters: shouldn't the compiler produce exactly the same code for these two variants by detecting the length 1 string? I thought this was possible in C++. I like code that is as uniform as possible, and using string literals for anything string-like seems simple and nice to me.
    $endgroup$
    – Roland Illig
    Apr 24 at 18:33











  • $begingroup$
    The compiler could in theory inline enough layers to get the same code in the end using whole-program-analysis, necessary due to virtual functions called. I doubt it though.
    $endgroup$
    – Deduplicator
    Apr 24 at 19:15










  • $begingroup$
    "You don't need the != NULL check before the delete[] since that pointer cannot be null" The behaviour is identical anyway, as the delete and delete[] operators are both defined to be null safe (in which case they do nothing).
    $endgroup$
    – Max Barraclough
    Apr 25 at 21:57










  • $begingroup$
    Thanks for the Review
    $endgroup$
    – f1r361rd
    2 days ago













18












18








18





$begingroup$

If you had tagged this code as C, it would have been acceptable. Since you tagged it as C++, it's horrible.



Instead of writing your own swap function, there's already std::swap in <algorithm>.



Instead of writing bubble sort yourself, just use std::sort, also from <algorithm>.



Instead of using arrays and resizing them yourself, just use std::vector<int>, from <vector>.



After applying these transformations, you cannot have a dangling pointer anymore since your code is completely pointer-free.



As part of an exercise for learning the basic operations on memory management, it's ok to write code like this, but never ever use such code in production. In production the code should look like this:



#include <algorithm>
#include <iostream>
#include <vector>

void Print(const std::vector<int> &nums)

for(int num : nums)

std::cout << " " << num;

std::cout << "n";


int main()

std::vector<int> nums 5, 7, 3, 1 ;

// There's probably a more elegant way to add the elements to the vector.
nums.push_back(4);
nums.push_back(2);
nums.push_back(8);

std::sort(nums.begin(), nums.end());

Print(nums);



By the way, your original code doesn't have any dangling pointer as well. Well done.



You don't need the != NULL check before the delete[] since that pointer cannot be null. In modern C++ (since C++11 I think) you would also write nullptr instead of NULL. The reason is that historically NULL had not been guaranteed to be of pointer type.



Have a look at https://en.cppreference.com/w/cpp/algorithm for more algorithms that you shouldn't implement yourself in C++.



I would have liked to write the push_back block in a shorter way, as well as the Print function. I'm sure there's a more elegant way, I just don't know it.






share|improve this answer











$endgroup$



If you had tagged this code as C, it would have been acceptable. Since you tagged it as C++, it's horrible.



Instead of writing your own swap function, there's already std::swap in <algorithm>.



Instead of writing bubble sort yourself, just use std::sort, also from <algorithm>.



Instead of using arrays and resizing them yourself, just use std::vector<int>, from <vector>.



After applying these transformations, you cannot have a dangling pointer anymore since your code is completely pointer-free.



As part of an exercise for learning the basic operations on memory management, it's ok to write code like this, but never ever use such code in production. In production the code should look like this:



#include <algorithm>
#include <iostream>
#include <vector>

void Print(const std::vector<int> &nums)

for(int num : nums)

std::cout << " " << num;

std::cout << "n";


int main()

std::vector<int> nums 5, 7, 3, 1 ;

// There's probably a more elegant way to add the elements to the vector.
nums.push_back(4);
nums.push_back(2);
nums.push_back(8);

std::sort(nums.begin(), nums.end());

Print(nums);



By the way, your original code doesn't have any dangling pointer as well. Well done.



You don't need the != NULL check before the delete[] since that pointer cannot be null. In modern C++ (since C++11 I think) you would also write nullptr instead of NULL. The reason is that historically NULL had not been guaranteed to be of pointer type.



Have a look at https://en.cppreference.com/w/cpp/algorithm for more algorithms that you shouldn't implement yourself in C++.



I would have liked to write the push_back block in a shorter way, as well as the Print function. I'm sure there's a more elegant way, I just don't know it.







share|improve this answer














share|improve this answer



share|improve this answer








edited Apr 24 at 18:29

























answered Apr 24 at 6:19









Roland IlligRoland Illig

12.4k12050




12.4k12050











  • $begingroup$
    I would just stream char-literals instead of length-one string-literals. And cppreference.com is inho a better reference.
    $endgroup$
    – Deduplicator
    Apr 24 at 11:45











  • $begingroup$
    @Deduplicator I changed the link, thanks for the hint. Regarding the single characters: shouldn't the compiler produce exactly the same code for these two variants by detecting the length 1 string? I thought this was possible in C++. I like code that is as uniform as possible, and using string literals for anything string-like seems simple and nice to me.
    $endgroup$
    – Roland Illig
    Apr 24 at 18:33











  • $begingroup$
    The compiler could in theory inline enough layers to get the same code in the end using whole-program-analysis, necessary due to virtual functions called. I doubt it though.
    $endgroup$
    – Deduplicator
    Apr 24 at 19:15










  • $begingroup$
    "You don't need the != NULL check before the delete[] since that pointer cannot be null" The behaviour is identical anyway, as the delete and delete[] operators are both defined to be null safe (in which case they do nothing).
    $endgroup$
    – Max Barraclough
    Apr 25 at 21:57










  • $begingroup$
    Thanks for the Review
    $endgroup$
    – f1r361rd
    2 days ago
















  • $begingroup$
    I would just stream char-literals instead of length-one string-literals. And cppreference.com is inho a better reference.
    $endgroup$
    – Deduplicator
    Apr 24 at 11:45











  • $begingroup$
    @Deduplicator I changed the link, thanks for the hint. Regarding the single characters: shouldn't the compiler produce exactly the same code for these two variants by detecting the length 1 string? I thought this was possible in C++. I like code that is as uniform as possible, and using string literals for anything string-like seems simple and nice to me.
    $endgroup$
    – Roland Illig
    Apr 24 at 18:33











  • $begingroup$
    The compiler could in theory inline enough layers to get the same code in the end using whole-program-analysis, necessary due to virtual functions called. I doubt it though.
    $endgroup$
    – Deduplicator
    Apr 24 at 19:15










  • $begingroup$
    "You don't need the != NULL check before the delete[] since that pointer cannot be null" The behaviour is identical anyway, as the delete and delete[] operators are both defined to be null safe (in which case they do nothing).
    $endgroup$
    – Max Barraclough
    Apr 25 at 21:57










  • $begingroup$
    Thanks for the Review
    $endgroup$
    – f1r361rd
    2 days ago















$begingroup$
I would just stream char-literals instead of length-one string-literals. And cppreference.com is inho a better reference.
$endgroup$
– Deduplicator
Apr 24 at 11:45





$begingroup$
I would just stream char-literals instead of length-one string-literals. And cppreference.com is inho a better reference.
$endgroup$
– Deduplicator
Apr 24 at 11:45













$begingroup$
@Deduplicator I changed the link, thanks for the hint. Regarding the single characters: shouldn't the compiler produce exactly the same code for these two variants by detecting the length 1 string? I thought this was possible in C++. I like code that is as uniform as possible, and using string literals for anything string-like seems simple and nice to me.
$endgroup$
– Roland Illig
Apr 24 at 18:33





$begingroup$
@Deduplicator I changed the link, thanks for the hint. Regarding the single characters: shouldn't the compiler produce exactly the same code for these two variants by detecting the length 1 string? I thought this was possible in C++. I like code that is as uniform as possible, and using string literals for anything string-like seems simple and nice to me.
$endgroup$
– Roland Illig
Apr 24 at 18:33













$begingroup$
The compiler could in theory inline enough layers to get the same code in the end using whole-program-analysis, necessary due to virtual functions called. I doubt it though.
$endgroup$
– Deduplicator
Apr 24 at 19:15




$begingroup$
The compiler could in theory inline enough layers to get the same code in the end using whole-program-analysis, necessary due to virtual functions called. I doubt it though.
$endgroup$
– Deduplicator
Apr 24 at 19:15












$begingroup$
"You don't need the != NULL check before the delete[] since that pointer cannot be null" The behaviour is identical anyway, as the delete and delete[] operators are both defined to be null safe (in which case they do nothing).
$endgroup$
– Max Barraclough
Apr 25 at 21:57




$begingroup$
"You don't need the != NULL check before the delete[] since that pointer cannot be null" The behaviour is identical anyway, as the delete and delete[] operators are both defined to be null safe (in which case they do nothing).
$endgroup$
– Max Barraclough
Apr 25 at 21:57












$begingroup$
Thanks for the Review
$endgroup$
– f1r361rd
2 days ago




$begingroup$
Thanks for the Review
$endgroup$
– f1r361rd
2 days ago













4












$begingroup$

The code is obviously wrong: your compiler should have warmed you that Resize() never uses its nSizeOld parameter.






share|improve this answer









$endgroup$








  • 1




    $begingroup$
    That's a funny concept of "wrong"... Anyway, it was probably a typo, since it actually is needed; the question was edited accordingly. This answer is superfluous now.
    $endgroup$
    – Anakhand
    Apr 24 at 6:58







  • 2




    $begingroup$
    Please see What to do when someone answers. I have rolled back Rev 2 → 1.
    $endgroup$
    – 200_success
    Apr 24 at 7:09






  • 4




    $begingroup$
    If we follow those rules literally, to correct [what seems to me] a simple typo, OP should ask a completely new question with the fix. Seems kind of ridiculous for such a trivial matter... If every question followed that rule verbatim this site would me a mess. I'm not saying your comment isn't correct or helpful, but it should be precisely that, a comment, so that OP can fix the [irrelevant] issue and move on.
    $endgroup$
    – Anakhand
    Apr 24 at 8:11







  • 2




    $begingroup$
    While your answer is correct and you made a good observation I'd contend that this type of error (a trivial bug) was not what the OP wanted to ask (it would, after all, be OT), so that it does not warrant an answer. I'd have written a comment and waited for the correction. You should imo reinstate the corrected version of the question and delete (or preface as obsolete) your answer.
    $endgroup$
    – Peter A. Schneider
    Apr 24 at 17:10







  • 1




    $begingroup$
    @PeterA.Schneider As per the rules in the help center, all aspects of the code are open to critique. This out-of-bounds memory access bug (which may or may not have been apparent to the author) is absolutely fair game for a Code Review answer.
    $endgroup$
    – 200_success
    Apr 24 at 17:17















4












$begingroup$

The code is obviously wrong: your compiler should have warmed you that Resize() never uses its nSizeOld parameter.






share|improve this answer









$endgroup$








  • 1




    $begingroup$
    That's a funny concept of "wrong"... Anyway, it was probably a typo, since it actually is needed; the question was edited accordingly. This answer is superfluous now.
    $endgroup$
    – Anakhand
    Apr 24 at 6:58







  • 2




    $begingroup$
    Please see What to do when someone answers. I have rolled back Rev 2 → 1.
    $endgroup$
    – 200_success
    Apr 24 at 7:09






  • 4




    $begingroup$
    If we follow those rules literally, to correct [what seems to me] a simple typo, OP should ask a completely new question with the fix. Seems kind of ridiculous for such a trivial matter... If every question followed that rule verbatim this site would me a mess. I'm not saying your comment isn't correct or helpful, but it should be precisely that, a comment, so that OP can fix the [irrelevant] issue and move on.
    $endgroup$
    – Anakhand
    Apr 24 at 8:11







  • 2




    $begingroup$
    While your answer is correct and you made a good observation I'd contend that this type of error (a trivial bug) was not what the OP wanted to ask (it would, after all, be OT), so that it does not warrant an answer. I'd have written a comment and waited for the correction. You should imo reinstate the corrected version of the question and delete (or preface as obsolete) your answer.
    $endgroup$
    – Peter A. Schneider
    Apr 24 at 17:10







  • 1




    $begingroup$
    @PeterA.Schneider As per the rules in the help center, all aspects of the code are open to critique. This out-of-bounds memory access bug (which may or may not have been apparent to the author) is absolutely fair game for a Code Review answer.
    $endgroup$
    – 200_success
    Apr 24 at 17:17













4












4








4





$begingroup$

The code is obviously wrong: your compiler should have warmed you that Resize() never uses its nSizeOld parameter.






share|improve this answer









$endgroup$



The code is obviously wrong: your compiler should have warmed you that Resize() never uses its nSizeOld parameter.







share|improve this answer












share|improve this answer



share|improve this answer










answered Apr 24 at 6:37









200_success200_success

132k20159424




132k20159424







  • 1




    $begingroup$
    That's a funny concept of "wrong"... Anyway, it was probably a typo, since it actually is needed; the question was edited accordingly. This answer is superfluous now.
    $endgroup$
    – Anakhand
    Apr 24 at 6:58







  • 2




    $begingroup$
    Please see What to do when someone answers. I have rolled back Rev 2 → 1.
    $endgroup$
    – 200_success
    Apr 24 at 7:09






  • 4




    $begingroup$
    If we follow those rules literally, to correct [what seems to me] a simple typo, OP should ask a completely new question with the fix. Seems kind of ridiculous for such a trivial matter... If every question followed that rule verbatim this site would me a mess. I'm not saying your comment isn't correct or helpful, but it should be precisely that, a comment, so that OP can fix the [irrelevant] issue and move on.
    $endgroup$
    – Anakhand
    Apr 24 at 8:11







  • 2




    $begingroup$
    While your answer is correct and you made a good observation I'd contend that this type of error (a trivial bug) was not what the OP wanted to ask (it would, after all, be OT), so that it does not warrant an answer. I'd have written a comment and waited for the correction. You should imo reinstate the corrected version of the question and delete (or preface as obsolete) your answer.
    $endgroup$
    – Peter A. Schneider
    Apr 24 at 17:10







  • 1




    $begingroup$
    @PeterA.Schneider As per the rules in the help center, all aspects of the code are open to critique. This out-of-bounds memory access bug (which may or may not have been apparent to the author) is absolutely fair game for a Code Review answer.
    $endgroup$
    – 200_success
    Apr 24 at 17:17












  • 1




    $begingroup$
    That's a funny concept of "wrong"... Anyway, it was probably a typo, since it actually is needed; the question was edited accordingly. This answer is superfluous now.
    $endgroup$
    – Anakhand
    Apr 24 at 6:58







  • 2




    $begingroup$
    Please see What to do when someone answers. I have rolled back Rev 2 → 1.
    $endgroup$
    – 200_success
    Apr 24 at 7:09






  • 4




    $begingroup$
    If we follow those rules literally, to correct [what seems to me] a simple typo, OP should ask a completely new question with the fix. Seems kind of ridiculous for such a trivial matter... If every question followed that rule verbatim this site would me a mess. I'm not saying your comment isn't correct or helpful, but it should be precisely that, a comment, so that OP can fix the [irrelevant] issue and move on.
    $endgroup$
    – Anakhand
    Apr 24 at 8:11







  • 2




    $begingroup$
    While your answer is correct and you made a good observation I'd contend that this type of error (a trivial bug) was not what the OP wanted to ask (it would, after all, be OT), so that it does not warrant an answer. I'd have written a comment and waited for the correction. You should imo reinstate the corrected version of the question and delete (or preface as obsolete) your answer.
    $endgroup$
    – Peter A. Schneider
    Apr 24 at 17:10







  • 1




    $begingroup$
    @PeterA.Schneider As per the rules in the help center, all aspects of the code are open to critique. This out-of-bounds memory access bug (which may or may not have been apparent to the author) is absolutely fair game for a Code Review answer.
    $endgroup$
    – 200_success
    Apr 24 at 17:17







1




1




$begingroup$
That's a funny concept of "wrong"... Anyway, it was probably a typo, since it actually is needed; the question was edited accordingly. This answer is superfluous now.
$endgroup$
– Anakhand
Apr 24 at 6:58





$begingroup$
That's a funny concept of "wrong"... Anyway, it was probably a typo, since it actually is needed; the question was edited accordingly. This answer is superfluous now.
$endgroup$
– Anakhand
Apr 24 at 6:58





2




2




$begingroup$
Please see What to do when someone answers. I have rolled back Rev 2 → 1.
$endgroup$
– 200_success
Apr 24 at 7:09




$begingroup$
Please see What to do when someone answers. I have rolled back Rev 2 → 1.
$endgroup$
– 200_success
Apr 24 at 7:09




4




4




$begingroup$
If we follow those rules literally, to correct [what seems to me] a simple typo, OP should ask a completely new question with the fix. Seems kind of ridiculous for such a trivial matter... If every question followed that rule verbatim this site would me a mess. I'm not saying your comment isn't correct or helpful, but it should be precisely that, a comment, so that OP can fix the [irrelevant] issue and move on.
$endgroup$
– Anakhand
Apr 24 at 8:11





$begingroup$
If we follow those rules literally, to correct [what seems to me] a simple typo, OP should ask a completely new question with the fix. Seems kind of ridiculous for such a trivial matter... If every question followed that rule verbatim this site would me a mess. I'm not saying your comment isn't correct or helpful, but it should be precisely that, a comment, so that OP can fix the [irrelevant] issue and move on.
$endgroup$
– Anakhand
Apr 24 at 8:11





2




2




$begingroup$
While your answer is correct and you made a good observation I'd contend that this type of error (a trivial bug) was not what the OP wanted to ask (it would, after all, be OT), so that it does not warrant an answer. I'd have written a comment and waited for the correction. You should imo reinstate the corrected version of the question and delete (or preface as obsolete) your answer.
$endgroup$
– Peter A. Schneider
Apr 24 at 17:10





$begingroup$
While your answer is correct and you made a good observation I'd contend that this type of error (a trivial bug) was not what the OP wanted to ask (it would, after all, be OT), so that it does not warrant an answer. I'd have written a comment and waited for the correction. You should imo reinstate the corrected version of the question and delete (or preface as obsolete) your answer.
$endgroup$
– Peter A. Schneider
Apr 24 at 17:10





1




1




$begingroup$
@PeterA.Schneider As per the rules in the help center, all aspects of the code are open to critique. This out-of-bounds memory access bug (which may or may not have been apparent to the author) is absolutely fair game for a Code Review answer.
$endgroup$
– 200_success
Apr 24 at 17:17




$begingroup$
@PeterA.Schneider As per the rules in the help center, all aspects of the code are open to critique. This out-of-bounds memory access bug (which may or may not have been apparent to the author) is absolutely fair game for a Code Review answer.
$endgroup$
– 200_success
Apr 24 at 17:17











2












$begingroup$

  1. Your code is too low-level. It expresses implementation details instead of intent. That's why your code looks like "C with couts instead of printf and new/delete instead of malloc/free" instead of C++.



  2. Roland Illig has already told you that you should use std::swap instead of building a new one from scratch. You should use existing libraries, especially the standard library, whenever possible.



    That said, your own implementation of swap is also questionable. This is C++, not C. We have references. Using pointers makes the code less readable, and puts burden on the user of the function. So you should change it to:




    void swap(int& x, int& y)

    int temp = x;
    x = y;
    y = temp;



    And the calls to it can be changed from swap(&foo, &bar) to swap(foo, bar). Still, std::swap is preferable.



  3. Again, Roland Illig has already told you that you should use the std::sort instead of building a new bubble sort from scratch. std::sort typically uses quicksort, which has $O(n log n)$ time complexity; whereas bubble sort has $O(n^2)$ time complexity. It should be obvious that std::sort is much more efficient.



  4. Your parameter lists are so C-ish. (pointer, size) parameter pairs are everywhere. They are error-prone. Consider using spans. (Spans are currently not available in the standard library; consider using the one from GSL)



    You even have parameter lists like (int*& Array, const int& nSizeOld, const int& nSize). Don't pass by const reference for builtin types. Just pass by value, as in int nSizeOld, int nSize. And letting a pointer denote an array with sizes littered everywhere holds a great welcome party for errors.




  5. Don't use _tmain and _TCHAR. They are not portable. (Strictly speaking, they are not proper C++.) You should write in ISO standard C++. Use main and char instead.




    // Correct prototype of the main function
    int main(int argc, char* argv[])

    // ...



  6. Don't make such liberal use of "naked" news and deletes. Explicit calls to news and deletes are very error prone. std::vectors should be preferred from the beginning.


  7. You have four for loops in total. The first three use i++, whereas the last one uses ++i. Please consistently use ++i.


As a conclusion: you should refactor your code to express intent.






share|improve this answer









$endgroup$












  • $begingroup$
    Thanks for the Review.
    $endgroup$
    – f1r361rd
    2 days ago















2












$begingroup$

  1. Your code is too low-level. It expresses implementation details instead of intent. That's why your code looks like "C with couts instead of printf and new/delete instead of malloc/free" instead of C++.



  2. Roland Illig has already told you that you should use std::swap instead of building a new one from scratch. You should use existing libraries, especially the standard library, whenever possible.



    That said, your own implementation of swap is also questionable. This is C++, not C. We have references. Using pointers makes the code less readable, and puts burden on the user of the function. So you should change it to:




    void swap(int& x, int& y)

    int temp = x;
    x = y;
    y = temp;



    And the calls to it can be changed from swap(&foo, &bar) to swap(foo, bar). Still, std::swap is preferable.



  3. Again, Roland Illig has already told you that you should use the std::sort instead of building a new bubble sort from scratch. std::sort typically uses quicksort, which has $O(n log n)$ time complexity; whereas bubble sort has $O(n^2)$ time complexity. It should be obvious that std::sort is much more efficient.



  4. Your parameter lists are so C-ish. (pointer, size) parameter pairs are everywhere. They are error-prone. Consider using spans. (Spans are currently not available in the standard library; consider using the one from GSL)



    You even have parameter lists like (int*& Array, const int& nSizeOld, const int& nSize). Don't pass by const reference for builtin types. Just pass by value, as in int nSizeOld, int nSize. And letting a pointer denote an array with sizes littered everywhere holds a great welcome party for errors.




  5. Don't use _tmain and _TCHAR. They are not portable. (Strictly speaking, they are not proper C++.) You should write in ISO standard C++. Use main and char instead.




    // Correct prototype of the main function
    int main(int argc, char* argv[])

    // ...



  6. Don't make such liberal use of "naked" news and deletes. Explicit calls to news and deletes are very error prone. std::vectors should be preferred from the beginning.


  7. You have four for loops in total. The first three use i++, whereas the last one uses ++i. Please consistently use ++i.


As a conclusion: you should refactor your code to express intent.






share|improve this answer









$endgroup$












  • $begingroup$
    Thanks for the Review.
    $endgroup$
    – f1r361rd
    2 days ago













2












2








2





$begingroup$

  1. Your code is too low-level. It expresses implementation details instead of intent. That's why your code looks like "C with couts instead of printf and new/delete instead of malloc/free" instead of C++.



  2. Roland Illig has already told you that you should use std::swap instead of building a new one from scratch. You should use existing libraries, especially the standard library, whenever possible.



    That said, your own implementation of swap is also questionable. This is C++, not C. We have references. Using pointers makes the code less readable, and puts burden on the user of the function. So you should change it to:




    void swap(int& x, int& y)

    int temp = x;
    x = y;
    y = temp;



    And the calls to it can be changed from swap(&foo, &bar) to swap(foo, bar). Still, std::swap is preferable.



  3. Again, Roland Illig has already told you that you should use the std::sort instead of building a new bubble sort from scratch. std::sort typically uses quicksort, which has $O(n log n)$ time complexity; whereas bubble sort has $O(n^2)$ time complexity. It should be obvious that std::sort is much more efficient.



  4. Your parameter lists are so C-ish. (pointer, size) parameter pairs are everywhere. They are error-prone. Consider using spans. (Spans are currently not available in the standard library; consider using the one from GSL)



    You even have parameter lists like (int*& Array, const int& nSizeOld, const int& nSize). Don't pass by const reference for builtin types. Just pass by value, as in int nSizeOld, int nSize. And letting a pointer denote an array with sizes littered everywhere holds a great welcome party for errors.




  5. Don't use _tmain and _TCHAR. They are not portable. (Strictly speaking, they are not proper C++.) You should write in ISO standard C++. Use main and char instead.




    // Correct prototype of the main function
    int main(int argc, char* argv[])

    // ...



  6. Don't make such liberal use of "naked" news and deletes. Explicit calls to news and deletes are very error prone. std::vectors should be preferred from the beginning.


  7. You have four for loops in total. The first three use i++, whereas the last one uses ++i. Please consistently use ++i.


As a conclusion: you should refactor your code to express intent.






share|improve this answer









$endgroup$



  1. Your code is too low-level. It expresses implementation details instead of intent. That's why your code looks like "C with couts instead of printf and new/delete instead of malloc/free" instead of C++.



  2. Roland Illig has already told you that you should use std::swap instead of building a new one from scratch. You should use existing libraries, especially the standard library, whenever possible.



    That said, your own implementation of swap is also questionable. This is C++, not C. We have references. Using pointers makes the code less readable, and puts burden on the user of the function. So you should change it to:




    void swap(int& x, int& y)

    int temp = x;
    x = y;
    y = temp;



    And the calls to it can be changed from swap(&foo, &bar) to swap(foo, bar). Still, std::swap is preferable.



  3. Again, Roland Illig has already told you that you should use the std::sort instead of building a new bubble sort from scratch. std::sort typically uses quicksort, which has $O(n log n)$ time complexity; whereas bubble sort has $O(n^2)$ time complexity. It should be obvious that std::sort is much more efficient.



  4. Your parameter lists are so C-ish. (pointer, size) parameter pairs are everywhere. They are error-prone. Consider using spans. (Spans are currently not available in the standard library; consider using the one from GSL)



    You even have parameter lists like (int*& Array, const int& nSizeOld, const int& nSize). Don't pass by const reference for builtin types. Just pass by value, as in int nSizeOld, int nSize. And letting a pointer denote an array with sizes littered everywhere holds a great welcome party for errors.




  5. Don't use _tmain and _TCHAR. They are not portable. (Strictly speaking, they are not proper C++.) You should write in ISO standard C++. Use main and char instead.




    // Correct prototype of the main function
    int main(int argc, char* argv[])

    // ...



  6. Don't make such liberal use of "naked" news and deletes. Explicit calls to news and deletes are very error prone. std::vectors should be preferred from the beginning.


  7. You have four for loops in total. The first three use i++, whereas the last one uses ++i. Please consistently use ++i.


As a conclusion: you should refactor your code to express intent.







share|improve this answer












share|improve this answer



share|improve this answer










answered Apr 27 at 2:12









L. F.L. F.

31012




31012











  • $begingroup$
    Thanks for the Review.
    $endgroup$
    – f1r361rd
    2 days ago
















  • $begingroup$
    Thanks for the Review.
    $endgroup$
    – f1r361rd
    2 days ago















$begingroup$
Thanks for the Review.
$endgroup$
– f1r361rd
2 days ago




$begingroup$
Thanks for the Review.
$endgroup$
– f1r361rd
2 days ago

















draft saved

draft discarded
















































Thanks for contributing an answer to Code Review Stack Exchange!


  • Please be sure to answer the question. Provide details and share your research!

But avoid


  • Asking for help, clarification, or responding to other answers.

  • Making statements based on opinion; back them up with references or personal experience.

Use MathJax to format equations. MathJax reference.


To learn more, see our tips on writing great answers.




draft saved


draft discarded














StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f218994%2farray-dynamic-resize-in-heap%23new-answer', 'question_page');

);

Post as a guest















Required, but never shown





















































Required, but never shown














Required, but never shown












Required, but never shown







Required, but never shown

































Required, but never shown














Required, but never shown












Required, but never shown







Required, but never shown







Popular posts from this blog

Wikipedia:Vital articles Мазмуну Biography - Өмүр баян Philosophy and psychology - Философия жана психология Religion - Дин Social sciences - Коомдук илимдер Language and literature - Тил жана адабият Science - Илим Technology - Технология Arts and recreation - Искусство жана эс алуу History and geography - Тарых жана география Навигация менюсу

Bruxelas-Capital Índice Historia | Composición | Situación lingüística | Clima | Cidades irmandadas | Notas | Véxase tamén | Menú de navegacióneO uso das linguas en Bruxelas e a situación do neerlandés"Rexión de Bruxelas Capital"o orixinalSitio da rexiónPáxina de Bruselas no sitio da Oficina de Promoción Turística de Valonia e BruxelasMapa Interactivo da Rexión de Bruxelas-CapitaleeWorldCat332144929079854441105155190212ID28008674080552-90000 0001 0666 3698n94104302ID540940339365017018237

What should I write in an apology letter, since I have decided not to join a company after accepting an offer letterShould I keep looking after accepting a job offer?What should I do when I've been verbally told I would get an offer letter, but still haven't gotten one after 4 weeks?Do I accept an offer from a company that I am not likely to join?New job hasn't confirmed starting date and I want to give current employer as much notice as possibleHow should I address my manager in my resignation letter?HR delayed background verification, now jobless as resignedNo email communication after accepting a formal written offer. How should I phrase the call?What should I do if after receiving a verbal offer letter I am informed that my written job offer is put on hold due to some internal issues?Should I inform the current employer that I am about to resign within 1-2 weeks since I have signed the offer letter and waiting for visa?What company will do, if I send their offer letter to another company