Coding Challenge Solution - Good RangeBit-twiddling for a custom image formatFinding complementary pairsCodeChef Matrix RotationSummation of Arithmetic Progression Modulo SeriesSPOJ “To and Fro”Hackerrank's Sherlock and ArrayInteger packs (and integer pack utilities) for template meta-programmingHackerRank, Sherlock and ArrayLCD Display Programming challenge solutionk-combinations in lexicographic order (using given algorithm)
Can you please explain this joke: "I'm going bananas is what I tell my bananas before I leave the house"?
Creating Fictional Slavic Place Names
Coding Challenge Solution - Good Range
Order by does not work as I expect
Why does my electric oven present the option of 40A and 50A breakers?
Joist hangers to use for rough cut 2x8 (2 3/4" x 8 3/4")?
The most awesome army: 80 men left and 81 returned. Is it true?
If a massive object like Jupiter flew past the Earth how close would it need to come to pull people off of the surface?
When was the word "ambigu" first used with the sense of "meal with all items served at the same time"?
Could a soul from the Soulmonger be restored in the ToA campaign after this event?
What is a simple, physical situation where complex numbers emerge naturally?
How should I push back against my job assigning "homework"?
How did the Zip Chip and RocketChip accelerators work for the Apple II?
Is there a way to save this session?
What's the most polite way to tell a manager "shut up and let me work"?
Did airlines fly their aircraft slower in response to oil prices in the 1970s?
Is it possible to kill all life on Earth?
If a problem only occurs randomly once in every N times on average, how many tests do I have to perform to be certain that it's now fixed?
Are there mythical creatures in the world of Game of Thrones?
Different PCB color ( is it different material? )
Rotated Position of Integers
Does a component pouch automatically contain components?
Can a helicopter mask itself from Radar?
How was Apollo supposed to rendezvous in the case of a lunar abort?
Coding Challenge Solution - Good Range
Bit-twiddling for a custom image formatFinding complementary pairsCodeChef Matrix RotationSummation of Arithmetic Progression Modulo SeriesSPOJ “To and Fro”Hackerrank's Sherlock and ArrayInteger packs (and integer pack utilities) for template meta-programmingHackerRank, Sherlock and ArrayLCD Display Programming challenge solutionk-combinations in lexicographic order (using given algorithm)
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty margin-bottom:0;
$begingroup$
This my solution to this Good Range Coding Challenge
There is a number space given from 1 to N. And there are M queries followed by that. In each query, we were given a number between 1 to N (both inclusive). We add these number one by one into a set.
Good range: A range in which there is exactly one element present from the set.
For each query, we need to find the good ranges. We need to return the sum of boundry of all good ranges.
Input:
First line will take two integer for input N and M.
Then following m lines would be numbers between 1 and N (both inclusive).
Output:
Following M lines contains sum of boudaries of good ranges.
Note:
Range can consist of single element and represented as (x-x) where boundary sum will be x+x.
Example:
Input:
10 4
2
5
7
9
Output:
11
18
30
46
Explaination:
step-1) set: 2
good range: (1-10)
sum: 1+10=11
step-2) set: 2 5
good range: (1-4), (3-10)
sum: 1+4+3+10=18
step-3) set: 2 5 7
good range: (1-4), (3-6), (6-10)
sum: 1+4+3+6+6+10=30
step-4) set: 2 5 7 9
good range: (1-4), (3-6), (6-8), (8-10)
sum: 1+4+3+6+6+8+8+10=46
#include <iostream>
#include <set>
using namespace std;
class Solution
public:
Solution(const unsigned int _N, const unsigned int _M) : N(_N), M(_M)
void solve()
for(unsigned int i=0; i<M; ++i)
unsigned int v;
cin >> v;
cout << "New Element = " << v << endl;
q.insert(v);
print_res();
cout << endl;
void print_res()
unsigned int left=1;
auto it=q.begin();
unsigned int last = *it;
for(++it; it!=q.end(); ++it)
const unsigned int curr = *it;
const unsigned int right = curr-1;
cout << "[" << left << ", "<< right << "] contains " << last << " and sum = " << (left+right) << endl;
left = last+1;
last = curr;
const unsigned right = N;
cout << "[" << left << ", "<< right << "] contains " << last << " and sum = " << (left+right) << endl;
private:
unsigned int N;
unsigned int M;
set<unsigned int> q;
;
int main()
// your code goes here
unsigned int N=0;
unsigned int M=0;
cin >> N >> M;
Solution sol(N,M);
sol.solve();
return 0;
Note: I am aware I'm returning more information than required by the problem description but I have chosen to do it to include also debugging information
c++ programming-challenge interval
$endgroup$
add a comment |
$begingroup$
This my solution to this Good Range Coding Challenge
There is a number space given from 1 to N. And there are M queries followed by that. In each query, we were given a number between 1 to N (both inclusive). We add these number one by one into a set.
Good range: A range in which there is exactly one element present from the set.
For each query, we need to find the good ranges. We need to return the sum of boundry of all good ranges.
Input:
First line will take two integer for input N and M.
Then following m lines would be numbers between 1 and N (both inclusive).
Output:
Following M lines contains sum of boudaries of good ranges.
Note:
Range can consist of single element and represented as (x-x) where boundary sum will be x+x.
Example:
Input:
10 4
2
5
7
9
Output:
11
18
30
46
Explaination:
step-1) set: 2
good range: (1-10)
sum: 1+10=11
step-2) set: 2 5
good range: (1-4), (3-10)
sum: 1+4+3+10=18
step-3) set: 2 5 7
good range: (1-4), (3-6), (6-10)
sum: 1+4+3+6+6+10=30
step-4) set: 2 5 7 9
good range: (1-4), (3-6), (6-8), (8-10)
sum: 1+4+3+6+6+8+8+10=46
#include <iostream>
#include <set>
using namespace std;
class Solution
public:
Solution(const unsigned int _N, const unsigned int _M) : N(_N), M(_M)
void solve()
for(unsigned int i=0; i<M; ++i)
unsigned int v;
cin >> v;
cout << "New Element = " << v << endl;
q.insert(v);
print_res();
cout << endl;
void print_res()
unsigned int left=1;
auto it=q.begin();
unsigned int last = *it;
for(++it; it!=q.end(); ++it)
const unsigned int curr = *it;
const unsigned int right = curr-1;
cout << "[" << left << ", "<< right << "] contains " << last << " and sum = " << (left+right) << endl;
left = last+1;
last = curr;
const unsigned right = N;
cout << "[" << left << ", "<< right << "] contains " << last << " and sum = " << (left+right) << endl;
private:
unsigned int N;
unsigned int M;
set<unsigned int> q;
;
int main()
// your code goes here
unsigned int N=0;
unsigned int M=0;
cin >> N >> M;
Solution sol(N,M);
sol.solve();
return 0;
Note: I am aware I'm returning more information than required by the problem description but I have chosen to do it to include also debugging information
c++ programming-challenge interval
$endgroup$
add a comment |
$begingroup$
This my solution to this Good Range Coding Challenge
There is a number space given from 1 to N. And there are M queries followed by that. In each query, we were given a number between 1 to N (both inclusive). We add these number one by one into a set.
Good range: A range in which there is exactly one element present from the set.
For each query, we need to find the good ranges. We need to return the sum of boundry of all good ranges.
Input:
First line will take two integer for input N and M.
Then following m lines would be numbers between 1 and N (both inclusive).
Output:
Following M lines contains sum of boudaries of good ranges.
Note:
Range can consist of single element and represented as (x-x) where boundary sum will be x+x.
Example:
Input:
10 4
2
5
7
9
Output:
11
18
30
46
Explaination:
step-1) set: 2
good range: (1-10)
sum: 1+10=11
step-2) set: 2 5
good range: (1-4), (3-10)
sum: 1+4+3+10=18
step-3) set: 2 5 7
good range: (1-4), (3-6), (6-10)
sum: 1+4+3+6+6+10=30
step-4) set: 2 5 7 9
good range: (1-4), (3-6), (6-8), (8-10)
sum: 1+4+3+6+6+8+8+10=46
#include <iostream>
#include <set>
using namespace std;
class Solution
public:
Solution(const unsigned int _N, const unsigned int _M) : N(_N), M(_M)
void solve()
for(unsigned int i=0; i<M; ++i)
unsigned int v;
cin >> v;
cout << "New Element = " << v << endl;
q.insert(v);
print_res();
cout << endl;
void print_res()
unsigned int left=1;
auto it=q.begin();
unsigned int last = *it;
for(++it; it!=q.end(); ++it)
const unsigned int curr = *it;
const unsigned int right = curr-1;
cout << "[" << left << ", "<< right << "] contains " << last << " and sum = " << (left+right) << endl;
left = last+1;
last = curr;
const unsigned right = N;
cout << "[" << left << ", "<< right << "] contains " << last << " and sum = " << (left+right) << endl;
private:
unsigned int N;
unsigned int M;
set<unsigned int> q;
;
int main()
// your code goes here
unsigned int N=0;
unsigned int M=0;
cin >> N >> M;
Solution sol(N,M);
sol.solve();
return 0;
Note: I am aware I'm returning more information than required by the problem description but I have chosen to do it to include also debugging information
c++ programming-challenge interval
$endgroup$
This my solution to this Good Range Coding Challenge
There is a number space given from 1 to N. And there are M queries followed by that. In each query, we were given a number between 1 to N (both inclusive). We add these number one by one into a set.
Good range: A range in which there is exactly one element present from the set.
For each query, we need to find the good ranges. We need to return the sum of boundry of all good ranges.
Input:
First line will take two integer for input N and M.
Then following m lines would be numbers between 1 and N (both inclusive).
Output:
Following M lines contains sum of boudaries of good ranges.
Note:
Range can consist of single element and represented as (x-x) where boundary sum will be x+x.
Example:
Input:
10 4
2
5
7
9
Output:
11
18
30
46
Explaination:
step-1) set: 2
good range: (1-10)
sum: 1+10=11
step-2) set: 2 5
good range: (1-4), (3-10)
sum: 1+4+3+10=18
step-3) set: 2 5 7
good range: (1-4), (3-6), (6-10)
sum: 1+4+3+6+6+10=30
step-4) set: 2 5 7 9
good range: (1-4), (3-6), (6-8), (8-10)
sum: 1+4+3+6+6+8+8+10=46
#include <iostream>
#include <set>
using namespace std;
class Solution
public:
Solution(const unsigned int _N, const unsigned int _M) : N(_N), M(_M)
void solve()
for(unsigned int i=0; i<M; ++i)
unsigned int v;
cin >> v;
cout << "New Element = " << v << endl;
q.insert(v);
print_res();
cout << endl;
void print_res()
unsigned int left=1;
auto it=q.begin();
unsigned int last = *it;
for(++it; it!=q.end(); ++it)
const unsigned int curr = *it;
const unsigned int right = curr-1;
cout << "[" << left << ", "<< right << "] contains " << last << " and sum = " << (left+right) << endl;
left = last+1;
last = curr;
const unsigned right = N;
cout << "[" << left << ", "<< right << "] contains " << last << " and sum = " << (left+right) << endl;
private:
unsigned int N;
unsigned int M;
set<unsigned int> q;
;
int main()
// your code goes here
unsigned int N=0;
unsigned int M=0;
cin >> N >> M;
Solution sol(N,M);
sol.solve();
return 0;
Note: I am aware I'm returning more information than required by the problem description but I have chosen to do it to include also debugging information
c++ programming-challenge interval
c++ programming-challenge interval
edited 7 hours ago
Harald Scheirich
1,434518
1,434518
asked 8 hours ago
Nicola BerniniNicola Bernini
1726
1726
add a comment |
add a comment |
2 Answers
2
active
oldest
votes
$begingroup$
Don't force a
class
where a function is perfectly fine.The namespace
std
is not designed for wholesale importation, see "Why is “using namespace std” considered bad practice?" for more detail.Identifiers starting with an underscore followed by an uppercase letter are reserved.
Input is generally unreliable. But you don't test for error at all.
Always recomputing the current result from first principles, instead of just updating it with the newest addition, is a waste of time.
Flushing is expensive, so avoid
std::endl
and explicitly request it withstd::flush
where unavoidable.You should probably follow the requested output-format...
You know that
int
inunsigned int
is implicit?In C++ and C99+,
return 0;
is implicit formain()
.I wonder why you sometimes surround binary operators with space, and sometimes don't.
Having over-long lines is very cumbersome to read. Admittedly, you need not make a hard cut at 79 nowadays.
As it is an interview question, one should verify that one should really use a Set, meaning that repeated elements have no effect, aside from causing an additional output.
As it is an interview question, one should verify that, contrary to the example, the inputs might not be sorted.
#include <cstdlib>
#include <iostream>
#include <set>
int main()
$endgroup$
add a comment |
$begingroup$
No comments on the algorithm itself. But I have many style improvements a interviewer probably looks for.
int main()
...
return 0;
Unlike in C the statement return 0
is automatically generated for main
. So its common to omit it.
then this line in the main is problematic as well:
unsigned int N = 0;
unsigned int M = 0;
cin >> N >> M;
You expect you get an unsigned integer from the input. But who says the user types it?
You should better read in as std::string
and convert the result to integer after if possible:
for (;;)
std::string input;
std::cin >> input;
if (is_convertible_to_integer(input)) // write a function to check if is convertible to unsigned int
// convert to string
break;
This can be probably in a function as well like unsigned int read_unsigend_int();
Then a classic mistake in C++. Don't use using namespace std
. It is not a good habit. Read about it here: https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice.
The next thing I wonder. Do you really need unsigned int? Often it is not worth the hassle. It can introduce hard to spot errors: https://stackoverflow.com/questions/22587451/c-c-use-of-int-or-unsigned-int
Then you create a class just for solving basically some computations. In this case I think its over-engineering. It could be simply solved by using free standing functions.
Unlike in some other Programming languages where everything is a class you can just use free standing functions.
However it is a good idea to wrap your functions and classes in its own namespace to prevent name clashes. So do something like:
namespace good_range
// youre functions and classes
Another small thing:
Solution(const unsigned int _N, const unsigned int _M) : N(_N), M(_M)
No need to use const here since you have the values by value copied anyway.
Also you should not use std::endl
for a newline. I even saw it in many wrong books. std::endl
gives you a newline and an expensive flushing operation of the buffer. Instead just use n
.
A opinion based thing. This line:
cout << "[" << left << ", " << right << "] contains " << last << " and sum = " << (left + right) << endl;
I would split it into two:
cout << "[" << left << ", " << right << "] contains " << last
<< " and sum = " << (left + right) << endl;
why? It is easier to read and you have the option to open two source files on one screen next to each other. I personally stick usually with 80 spaces per line. But some people user 100 or 120. To see a discussion about it:
https://stackoverflow.com/questions/276022/line-width-formatting-standard
$endgroup$
$begingroup$
Thanks for your feedback I appreciate all the comments make sense but let me say here my focus was on solving the algorithmic challenge so I have not paid much attention to good practices
$endgroup$
– Nicola Bernini
7 hours ago
$begingroup$
im sorry i don't have the time at the moment to check the algorithm. But I still wanted to contribute the style stuff I could spot right away. I think is alway important to write a good style. It shows youre professionalism.
$endgroup$
– Sandro4912
7 hours ago
2
$begingroup$
@NicolaBernini Hint: do not compute the sum from scratch every time; each insertion affects at most two ranges, and the sum can be updated in a constant time.
$endgroup$
– vnp
6 hours ago
add a comment |
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
);
);
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f221289%2fcoding-challenge-solution-good-range%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
$begingroup$
Don't force a
class
where a function is perfectly fine.The namespace
std
is not designed for wholesale importation, see "Why is “using namespace std” considered bad practice?" for more detail.Identifiers starting with an underscore followed by an uppercase letter are reserved.
Input is generally unreliable. But you don't test for error at all.
Always recomputing the current result from first principles, instead of just updating it with the newest addition, is a waste of time.
Flushing is expensive, so avoid
std::endl
and explicitly request it withstd::flush
where unavoidable.You should probably follow the requested output-format...
You know that
int
inunsigned int
is implicit?In C++ and C99+,
return 0;
is implicit formain()
.I wonder why you sometimes surround binary operators with space, and sometimes don't.
Having over-long lines is very cumbersome to read. Admittedly, you need not make a hard cut at 79 nowadays.
As it is an interview question, one should verify that one should really use a Set, meaning that repeated elements have no effect, aside from causing an additional output.
As it is an interview question, one should verify that, contrary to the example, the inputs might not be sorted.
#include <cstdlib>
#include <iostream>
#include <set>
int main()
$endgroup$
add a comment |
$begingroup$
Don't force a
class
where a function is perfectly fine.The namespace
std
is not designed for wholesale importation, see "Why is “using namespace std” considered bad practice?" for more detail.Identifiers starting with an underscore followed by an uppercase letter are reserved.
Input is generally unreliable. But you don't test for error at all.
Always recomputing the current result from first principles, instead of just updating it with the newest addition, is a waste of time.
Flushing is expensive, so avoid
std::endl
and explicitly request it withstd::flush
where unavoidable.You should probably follow the requested output-format...
You know that
int
inunsigned int
is implicit?In C++ and C99+,
return 0;
is implicit formain()
.I wonder why you sometimes surround binary operators with space, and sometimes don't.
Having over-long lines is very cumbersome to read. Admittedly, you need not make a hard cut at 79 nowadays.
As it is an interview question, one should verify that one should really use a Set, meaning that repeated elements have no effect, aside from causing an additional output.
As it is an interview question, one should verify that, contrary to the example, the inputs might not be sorted.
#include <cstdlib>
#include <iostream>
#include <set>
int main()
$endgroup$
add a comment |
$begingroup$
Don't force a
class
where a function is perfectly fine.The namespace
std
is not designed for wholesale importation, see "Why is “using namespace std” considered bad practice?" for more detail.Identifiers starting with an underscore followed by an uppercase letter are reserved.
Input is generally unreliable. But you don't test for error at all.
Always recomputing the current result from first principles, instead of just updating it with the newest addition, is a waste of time.
Flushing is expensive, so avoid
std::endl
and explicitly request it withstd::flush
where unavoidable.You should probably follow the requested output-format...
You know that
int
inunsigned int
is implicit?In C++ and C99+,
return 0;
is implicit formain()
.I wonder why you sometimes surround binary operators with space, and sometimes don't.
Having over-long lines is very cumbersome to read. Admittedly, you need not make a hard cut at 79 nowadays.
As it is an interview question, one should verify that one should really use a Set, meaning that repeated elements have no effect, aside from causing an additional output.
As it is an interview question, one should verify that, contrary to the example, the inputs might not be sorted.
#include <cstdlib>
#include <iostream>
#include <set>
int main()
$endgroup$
Don't force a
class
where a function is perfectly fine.The namespace
std
is not designed for wholesale importation, see "Why is “using namespace std” considered bad practice?" for more detail.Identifiers starting with an underscore followed by an uppercase letter are reserved.
Input is generally unreliable. But you don't test for error at all.
Always recomputing the current result from first principles, instead of just updating it with the newest addition, is a waste of time.
Flushing is expensive, so avoid
std::endl
and explicitly request it withstd::flush
where unavoidable.You should probably follow the requested output-format...
You know that
int
inunsigned int
is implicit?In C++ and C99+,
return 0;
is implicit formain()
.I wonder why you sometimes surround binary operators with space, and sometimes don't.
Having over-long lines is very cumbersome to read. Admittedly, you need not make a hard cut at 79 nowadays.
As it is an interview question, one should verify that one should really use a Set, meaning that repeated elements have no effect, aside from causing an additional output.
As it is an interview question, one should verify that, contrary to the example, the inputs might not be sorted.
#include <cstdlib>
#include <iostream>
#include <set>
int main()
edited 4 hours ago
answered 6 hours ago
DeduplicatorDeduplicator
12.5k2052
12.5k2052
add a comment |
add a comment |
$begingroup$
No comments on the algorithm itself. But I have many style improvements a interviewer probably looks for.
int main()
...
return 0;
Unlike in C the statement return 0
is automatically generated for main
. So its common to omit it.
then this line in the main is problematic as well:
unsigned int N = 0;
unsigned int M = 0;
cin >> N >> M;
You expect you get an unsigned integer from the input. But who says the user types it?
You should better read in as std::string
and convert the result to integer after if possible:
for (;;)
std::string input;
std::cin >> input;
if (is_convertible_to_integer(input)) // write a function to check if is convertible to unsigned int
// convert to string
break;
This can be probably in a function as well like unsigned int read_unsigend_int();
Then a classic mistake in C++. Don't use using namespace std
. It is not a good habit. Read about it here: https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice.
The next thing I wonder. Do you really need unsigned int? Often it is not worth the hassle. It can introduce hard to spot errors: https://stackoverflow.com/questions/22587451/c-c-use-of-int-or-unsigned-int
Then you create a class just for solving basically some computations. In this case I think its over-engineering. It could be simply solved by using free standing functions.
Unlike in some other Programming languages where everything is a class you can just use free standing functions.
However it is a good idea to wrap your functions and classes in its own namespace to prevent name clashes. So do something like:
namespace good_range
// youre functions and classes
Another small thing:
Solution(const unsigned int _N, const unsigned int _M) : N(_N), M(_M)
No need to use const here since you have the values by value copied anyway.
Also you should not use std::endl
for a newline. I even saw it in many wrong books. std::endl
gives you a newline and an expensive flushing operation of the buffer. Instead just use n
.
A opinion based thing. This line:
cout << "[" << left << ", " << right << "] contains " << last << " and sum = " << (left + right) << endl;
I would split it into two:
cout << "[" << left << ", " << right << "] contains " << last
<< " and sum = " << (left + right) << endl;
why? It is easier to read and you have the option to open two source files on one screen next to each other. I personally stick usually with 80 spaces per line. But some people user 100 or 120. To see a discussion about it:
https://stackoverflow.com/questions/276022/line-width-formatting-standard
$endgroup$
$begingroup$
Thanks for your feedback I appreciate all the comments make sense but let me say here my focus was on solving the algorithmic challenge so I have not paid much attention to good practices
$endgroup$
– Nicola Bernini
7 hours ago
$begingroup$
im sorry i don't have the time at the moment to check the algorithm. But I still wanted to contribute the style stuff I could spot right away. I think is alway important to write a good style. It shows youre professionalism.
$endgroup$
– Sandro4912
7 hours ago
2
$begingroup$
@NicolaBernini Hint: do not compute the sum from scratch every time; each insertion affects at most two ranges, and the sum can be updated in a constant time.
$endgroup$
– vnp
6 hours ago
add a comment |
$begingroup$
No comments on the algorithm itself. But I have many style improvements a interviewer probably looks for.
int main()
...
return 0;
Unlike in C the statement return 0
is automatically generated for main
. So its common to omit it.
then this line in the main is problematic as well:
unsigned int N = 0;
unsigned int M = 0;
cin >> N >> M;
You expect you get an unsigned integer from the input. But who says the user types it?
You should better read in as std::string
and convert the result to integer after if possible:
for (;;)
std::string input;
std::cin >> input;
if (is_convertible_to_integer(input)) // write a function to check if is convertible to unsigned int
// convert to string
break;
This can be probably in a function as well like unsigned int read_unsigend_int();
Then a classic mistake in C++. Don't use using namespace std
. It is not a good habit. Read about it here: https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice.
The next thing I wonder. Do you really need unsigned int? Often it is not worth the hassle. It can introduce hard to spot errors: https://stackoverflow.com/questions/22587451/c-c-use-of-int-or-unsigned-int
Then you create a class just for solving basically some computations. In this case I think its over-engineering. It could be simply solved by using free standing functions.
Unlike in some other Programming languages where everything is a class you can just use free standing functions.
However it is a good idea to wrap your functions and classes in its own namespace to prevent name clashes. So do something like:
namespace good_range
// youre functions and classes
Another small thing:
Solution(const unsigned int _N, const unsigned int _M) : N(_N), M(_M)
No need to use const here since you have the values by value copied anyway.
Also you should not use std::endl
for a newline. I even saw it in many wrong books. std::endl
gives you a newline and an expensive flushing operation of the buffer. Instead just use n
.
A opinion based thing. This line:
cout << "[" << left << ", " << right << "] contains " << last << " and sum = " << (left + right) << endl;
I would split it into two:
cout << "[" << left << ", " << right << "] contains " << last
<< " and sum = " << (left + right) << endl;
why? It is easier to read and you have the option to open two source files on one screen next to each other. I personally stick usually with 80 spaces per line. But some people user 100 or 120. To see a discussion about it:
https://stackoverflow.com/questions/276022/line-width-formatting-standard
$endgroup$
$begingroup$
Thanks for your feedback I appreciate all the comments make sense but let me say here my focus was on solving the algorithmic challenge so I have not paid much attention to good practices
$endgroup$
– Nicola Bernini
7 hours ago
$begingroup$
im sorry i don't have the time at the moment to check the algorithm. But I still wanted to contribute the style stuff I could spot right away. I think is alway important to write a good style. It shows youre professionalism.
$endgroup$
– Sandro4912
7 hours ago
2
$begingroup$
@NicolaBernini Hint: do not compute the sum from scratch every time; each insertion affects at most two ranges, and the sum can be updated in a constant time.
$endgroup$
– vnp
6 hours ago
add a comment |
$begingroup$
No comments on the algorithm itself. But I have many style improvements a interviewer probably looks for.
int main()
...
return 0;
Unlike in C the statement return 0
is automatically generated for main
. So its common to omit it.
then this line in the main is problematic as well:
unsigned int N = 0;
unsigned int M = 0;
cin >> N >> M;
You expect you get an unsigned integer from the input. But who says the user types it?
You should better read in as std::string
and convert the result to integer after if possible:
for (;;)
std::string input;
std::cin >> input;
if (is_convertible_to_integer(input)) // write a function to check if is convertible to unsigned int
// convert to string
break;
This can be probably in a function as well like unsigned int read_unsigend_int();
Then a classic mistake in C++. Don't use using namespace std
. It is not a good habit. Read about it here: https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice.
The next thing I wonder. Do you really need unsigned int? Often it is not worth the hassle. It can introduce hard to spot errors: https://stackoverflow.com/questions/22587451/c-c-use-of-int-or-unsigned-int
Then you create a class just for solving basically some computations. In this case I think its over-engineering. It could be simply solved by using free standing functions.
Unlike in some other Programming languages where everything is a class you can just use free standing functions.
However it is a good idea to wrap your functions and classes in its own namespace to prevent name clashes. So do something like:
namespace good_range
// youre functions and classes
Another small thing:
Solution(const unsigned int _N, const unsigned int _M) : N(_N), M(_M)
No need to use const here since you have the values by value copied anyway.
Also you should not use std::endl
for a newline. I even saw it in many wrong books. std::endl
gives you a newline and an expensive flushing operation of the buffer. Instead just use n
.
A opinion based thing. This line:
cout << "[" << left << ", " << right << "] contains " << last << " and sum = " << (left + right) << endl;
I would split it into two:
cout << "[" << left << ", " << right << "] contains " << last
<< " and sum = " << (left + right) << endl;
why? It is easier to read and you have the option to open two source files on one screen next to each other. I personally stick usually with 80 spaces per line. But some people user 100 or 120. To see a discussion about it:
https://stackoverflow.com/questions/276022/line-width-formatting-standard
$endgroup$
No comments on the algorithm itself. But I have many style improvements a interviewer probably looks for.
int main()
...
return 0;
Unlike in C the statement return 0
is automatically generated for main
. So its common to omit it.
then this line in the main is problematic as well:
unsigned int N = 0;
unsigned int M = 0;
cin >> N >> M;
You expect you get an unsigned integer from the input. But who says the user types it?
You should better read in as std::string
and convert the result to integer after if possible:
for (;;)
std::string input;
std::cin >> input;
if (is_convertible_to_integer(input)) // write a function to check if is convertible to unsigned int
// convert to string
break;
This can be probably in a function as well like unsigned int read_unsigend_int();
Then a classic mistake in C++. Don't use using namespace std
. It is not a good habit. Read about it here: https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice.
The next thing I wonder. Do you really need unsigned int? Often it is not worth the hassle. It can introduce hard to spot errors: https://stackoverflow.com/questions/22587451/c-c-use-of-int-or-unsigned-int
Then you create a class just for solving basically some computations. In this case I think its over-engineering. It could be simply solved by using free standing functions.
Unlike in some other Programming languages where everything is a class you can just use free standing functions.
However it is a good idea to wrap your functions and classes in its own namespace to prevent name clashes. So do something like:
namespace good_range
// youre functions and classes
Another small thing:
Solution(const unsigned int _N, const unsigned int _M) : N(_N), M(_M)
No need to use const here since you have the values by value copied anyway.
Also you should not use std::endl
for a newline. I even saw it in many wrong books. std::endl
gives you a newline and an expensive flushing operation of the buffer. Instead just use n
.
A opinion based thing. This line:
cout << "[" << left << ", " << right << "] contains " << last << " and sum = " << (left + right) << endl;
I would split it into two:
cout << "[" << left << ", " << right << "] contains " << last
<< " and sum = " << (left + right) << endl;
why? It is easier to read and you have the option to open two source files on one screen next to each other. I personally stick usually with 80 spaces per line. But some people user 100 or 120. To see a discussion about it:
https://stackoverflow.com/questions/276022/line-width-formatting-standard
edited 6 hours ago
Jerry Coffin
28.9k462130
28.9k462130
answered 7 hours ago
Sandro4912Sandro4912
1,5391426
1,5391426
$begingroup$
Thanks for your feedback I appreciate all the comments make sense but let me say here my focus was on solving the algorithmic challenge so I have not paid much attention to good practices
$endgroup$
– Nicola Bernini
7 hours ago
$begingroup$
im sorry i don't have the time at the moment to check the algorithm. But I still wanted to contribute the style stuff I could spot right away. I think is alway important to write a good style. It shows youre professionalism.
$endgroup$
– Sandro4912
7 hours ago
2
$begingroup$
@NicolaBernini Hint: do not compute the sum from scratch every time; each insertion affects at most two ranges, and the sum can be updated in a constant time.
$endgroup$
– vnp
6 hours ago
add a comment |
$begingroup$
Thanks for your feedback I appreciate all the comments make sense but let me say here my focus was on solving the algorithmic challenge so I have not paid much attention to good practices
$endgroup$
– Nicola Bernini
7 hours ago
$begingroup$
im sorry i don't have the time at the moment to check the algorithm. But I still wanted to contribute the style stuff I could spot right away. I think is alway important to write a good style. It shows youre professionalism.
$endgroup$
– Sandro4912
7 hours ago
2
$begingroup$
@NicolaBernini Hint: do not compute the sum from scratch every time; each insertion affects at most two ranges, and the sum can be updated in a constant time.
$endgroup$
– vnp
6 hours ago
$begingroup$
Thanks for your feedback I appreciate all the comments make sense but let me say here my focus was on solving the algorithmic challenge so I have not paid much attention to good practices
$endgroup$
– Nicola Bernini
7 hours ago
$begingroup$
Thanks for your feedback I appreciate all the comments make sense but let me say here my focus was on solving the algorithmic challenge so I have not paid much attention to good practices
$endgroup$
– Nicola Bernini
7 hours ago
$begingroup$
im sorry i don't have the time at the moment to check the algorithm. But I still wanted to contribute the style stuff I could spot right away. I think is alway important to write a good style. It shows youre professionalism.
$endgroup$
– Sandro4912
7 hours ago
$begingroup$
im sorry i don't have the time at the moment to check the algorithm. But I still wanted to contribute the style stuff I could spot right away. I think is alway important to write a good style. It shows youre professionalism.
$endgroup$
– Sandro4912
7 hours ago
2
2
$begingroup$
@NicolaBernini Hint: do not compute the sum from scratch every time; each insertion affects at most two ranges, and the sum can be updated in a constant time.
$endgroup$
– vnp
6 hours ago
$begingroup$
@NicolaBernini Hint: do not compute the sum from scratch every time; each insertion affects at most two ranges, and the sum can be updated in a constant time.
$endgroup$
– vnp
6 hours ago
add a comment |
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.
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f221289%2fcoding-challenge-solution-good-range%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
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