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

Club Baloncesto Breogán Índice Historia | Pavillón | Nome | O Breogán na cultura popular | Xogadores | Adestradores | Presidentes | Palmarés | Historial | Líderes | Notas | Véxase tamén | Menú de navegacióncbbreogan.galCadroGuía oficial da ACB 2009-10, páxina 201Guía oficial ACB 1992, páxina 183. Editorial DB.É de 6.500 espectadores sentados axeitándose á última normativa"Estudiantes Junior, entre as mellores canteiras"o orixinalHemeroteca El Mundo Deportivo, 16 setembro de 1970, páxina 12Historia do BreogánAlfredo Pérez, o último canoneiroHistoria C.B. BreogánHemeroteca de El Mundo DeportivoJimmy Wright, norteamericano do Breogán deixará Lugo por ameazas de morteResultados de Breogán en 1986-87Resultados de Breogán en 1990-91Ficha de Velimir Perasović en acb.comResultados de Breogán en 1994-95Breogán arrasa al Barça. "El Mundo Deportivo", 27 de setembro de 1999, páxina 58CB Breogán - FC BarcelonaA FEB invita a participar nunha nova Liga EuropeaCharlie Bell na prensa estatalMáximos anotadores 2005Tempada 2005-06 : Tódolos Xogadores da Xornada""Non quero pensar nunha man negra, mais pregúntome que está a pasar""o orixinalRaúl López, orgulloso dos xogadores, presume da boa saúde económica do BreogánJulio González confirma que cesa como presidente del BreogánHomenaxe a Lisardo GómezA tempada do rexurdimento celesteEntrevista a Lisardo GómezEl COB dinamita el Pazo para forzar el quinto (69-73)Cafés Candelas, patrocinador del CB Breogán"Suso Lázare, novo presidente do Breogán"o orixinalCafés Candelas Breogán firma el mayor triunfo de la historiaEl Breogán realizará 17 homenajes por su cincuenta aniversario"O Breogán honra ao seu fundador e primeiro presidente"o orixinalMiguel Giao recibiu a homenaxe do PazoHomenaxe aos primeiros gladiadores celestesO home que nos amosa como ver o Breo co corazónTita Franco será homenaxeada polos #50anosdeBreoJulio Vila recibirá unha homenaxe in memoriam polos #50anosdeBreo"O Breogán homenaxeará aos seus aboados máis veteráns"Pechada ovación a «Capi» Sanmartín e Ricardo «Corazón de González»Homenaxe por décadas de informaciónPaco García volve ao Pazo con motivo do 50 aniversario"Resultados y clasificaciones""O Cafés Candelas Breogán, campión da Copa Princesa""O Cafés Candelas Breogán, equipo ACB"C.B. Breogán"Proxecto social"o orixinal"Centros asociados"o orixinalFicha en imdb.comMario Camus trata la recuperación del amor en 'La vieja música', su última película"Páxina web oficial""Club Baloncesto Breogán""C. B. Breogán S.A.D."eehttp://www.fegaba.com

Vilaño, A Laracha Índice Patrimonio | Lugares e parroquias | Véxase tamén | Menú de navegación43°14′52″N 8°36′03″O / 43.24775, -8.60070

Cegueira Índice Epidemioloxía | Deficiencia visual | Tipos de cegueira | Principais causas de cegueira | Tratamento | Técnicas de adaptación e axudas | Vida dos cegos | Primeiros auxilios | Crenzas respecto das persoas cegas | Crenzas das persoas cegas | O neno deficiente visual | Aspectos psicolóxicos da cegueira | Notas | Véxase tamén | Menú de navegación54.054.154.436928256blindnessDicionario da Real Academia GalegaPortal das Palabras"International Standards: Visual Standards — Aspects and Ranges of Vision Loss with Emphasis on Population Surveys.""Visual impairment and blindness""Presentan un plan para previr a cegueira"o orixinalACCDV Associació Catalana de Cecs i Disminuïts Visuals - PMFTrachoma"Effect of gene therapy on visual function in Leber's congenital amaurosis"1844137110.1056/NEJMoa0802268Cans guía - os mellores amigos dos cegosArquivadoEscola de cans guía para cegos en Mortágua, PortugalArquivado"Tecnología para ciegos y deficientes visuales. Recopilación de recursos gratuitos en la Red""Colorino""‘COL.diesis’, escuchar los sonidos del color""COL.diesis: Transforming Colour into Melody and Implementing the Result in a Colour Sensor Device"o orixinal"Sistema de desarrollo de sinestesia color-sonido para invidentes utilizando un protocolo de audio""Enseñanza táctil - geometría y color. Juegos didácticos para niños ciegos y videntes""Sistema Constanz"L'ocupació laboral dels cecs a l'Estat espanyol està pràcticament equiparada a la de les persones amb visió, entrevista amb Pedro ZuritaONCE (Organización Nacional de Cegos de España)Prevención da cegueiraDescrición de deficiencias visuais (Disc@pnet)Braillín, un boneco atractivo para calquera neno, con ou sen discapacidade, que permite familiarizarse co sistema de escritura e lectura brailleAxudas Técnicas36838ID00897494007150-90057129528256DOID:1432HP:0000618D001766C10.597.751.941.162C97109C0155020