Return to Answer

• Indentation

Your indentation is not consistent. This makes the code hard to read and maintain. It should be fixed so you don't give other people headaches.

 if(left < right){
 while(l < nums1.size() && nums1[l] == left )l++;
 }else while( r < nums2.size() && nums2[r] == right )r++; 

That is basically unreadable giberish (opinion of Martin).

• Using namespace std; is super bad

This is mention in nearly every C++ review. There is a large article on the subject here: Why is "using namespace std" considered bad practice? the second answer is the best in my opinion (Martin) see

• Multiple declarations in one is bad (thanks to terrible syntax binding rules)

The one declaration per line has been written about adnausium in best practice guides. Please for the sake of your reader declare one variable per line with its own exact type.

The syntax binding rules alluded to above is:

int* x, y; // here x is int* and y in int
 // confusing to a reader. Did you really mean to make y an int?
 // Avoid this problem be declaring one variable per line

• Typically, functions like this would be based on iterators to work on any container

Here your code is limited to only using vectors. But the algorithm you are using could be used by any container type with only small modifications. As a result your function could provide mcuh more utility being written to use iterators.

The standard library was written such that iterators are the glue between algorithms and container.

• It would be a lot simpler if not necessarily more efficient at runtime to just use some hash sets
• This function could be generic in T rather than assuming int
• The repeated conditions make me feel like there's simplification waiting here, although exactly what that is eludes me in the two minutes I'm spending on this
• Should take by const ref, not ref, so that you can operate on temporaries

• Indentation

  Your indentation is not consistent. This makes the code hard to read and maintain. It should be fixed so you don't give other people headaches.

   if(left < right){
   while(l < nums1.size() && nums1[l] == left )l++;
   }else while( r < nums2.size() && nums2[r] == right )r++;
  

  That is basically unreadable giberish (opinion of Martin).

• Using namespace std; is super bad

  This is mention in nearly every C++ review. There is a large article on the subject here: Why is "using namespace std" considered bad practice? . The second answer is the best in my opinion (Martin) see

• Multiple declarations in one is bad (thanks to terrible syntax binding rules)

  The one declaration per line has been written about adnausium in best practice guides. Please for the sake of your reader declare one variable per line with its own exact type.

  The syntax binding rules alluded to above is:

   int* x, y; // Here x is int* and y in int
   // confusing to a reader. Did you really mean to make y an int?
   // Avoid this problem be declaring one variable per line
  

• Typically, functions like this would be based on iterators to work on any container

  Here your code is limited to only using vectors. But the algorithm you are using could be used by any container type with only small modifications. As a result your function could provide much more utility being written to use iterators.

  The standard library was written such that iterators are the glue between algorithms and container.

• It would be a lot simpler, if not necessarily more efficient at runtime, to just use some hash sets.

• This function could be generic in T rather than assuming int.

• The repeated conditions make me feel like there's simplification waiting here, although exactly what that is eludes me in the two minutes I'm spending on this.

• Should take by const ref, not ref, so that you can operate on temporaries.

• Indentation
• Using namespace std; is super bad
• Multiple declarations in one is bad (thanks to terrible syntax binding rules)
• Typically, functions like this would be based on iterators to work on any container
• It would be a lot simpler if not necessarily more efficient at runtime to just use some hash sets
• This function could be generic in T rather than assuming int
• The repeated conditions make me feel like there's simplification waiting here, although exactly what that is eludes me in the two minutes I'm spending on this
• Should take by const ref, not ref, so that you can operate on temporaries

• Indentation

Your indentation is not consistent. This makes the code hard to read and maintain. It should be fixed so you don't give other people headaches.

 if(left < right){
 while(l < nums1.size() && nums1[l] == left )l++;
 }else while( r < nums2.size() && nums2[r] == right )r++; 

That is basically unreadable giberish (opinion of Martin).

• Using namespace std; is super bad

This is mention in nearly every C++ review. There is a large article on the subject here: Why is "using namespace std" considered bad practice? the second answer is the best in my opinion (Martin) see

• Multiple declarations in one is bad (thanks to terrible syntax binding rules)

The one declaration per line has been written about adnausium in best practice guides. Please for the sake of your reader declare one variable per line with its own exact type.

The syntax binding rules alluded to above is:

int* x, y; // here x is int* and y in int
 // confusing to a reader. Did you really mean to make y an int?
 // Avoid this problem be declaring one variable per line

• Typically, functions like this would be based on iterators to work on any container

Here your code is limited to only using vectors. But the algorithm you are using could be used by any container type with only small modifications. As a result your function could provide mcuh more utility being written to use iterators.

The standard library was written such that iterators are the glue between algorithms and container.

• It would be a lot simpler if not necessarily more efficient at runtime to just use some hash sets
• This function could be generic in T rather than assuming int
• The repeated conditions make me feel like there's simplification waiting here, although exactly what that is eludes me in the two minutes I'm spending on this
• Should take by const ref, not ref, so that you can operate on temporaries