Coding Challenge Solution - Good RangeBit-twiddling for a custom image formatFinding complementary...
Estimate related to the Möbius function
Is there any Biblical Basis for 400 years of silence between Old and New Testament?
What caused the tendency for conservatives to not support climate change regulations?
Why is there a need to modify system call tables in Linux?
What is the most important characteristic of New Weird as a genre?
How do I get a list of only the files (not the directories) from a package?
What are the problems in teaching guitar via Skype?
Is there an evolutionary advantage to having two heads?
Could a guilty Boris Johnson be used to cancel Brexit?
Why were the Night's Watch required to be celibate?
Can you please explain this joke: "I'm going bananas is what I tell my bananas before I leave the house"?
Why does my electric oven present the option of 40A and 50A breakers?
Can an old DSLR be upgraded to match modern smartphone image quality
Scala list with same adjacent values
Make a formula to get the highest score
Select * from View takes 4 minutes
California: "For quality assurance, this phone call is being recorded"
Why would Lupin kill Pettigrew?
Asking for something with different prices
What's the most polite way to tell a manager "shut up and let me work"?
Future enhancements for the finite element method
Singlequote and backslash
Order by does not work as I expect
Humans meet a distant alien species. How do they standardize? - Units of Measure
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 8 hours ago
Harald Scheirich
1,434518
1,434518
asked 9 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() {
unsigned n, m, x;
std::cin >> n >> m;
if (!std::cin || !n) {
std::cerr << "Bad Input!n";
return EXIT_FAILURE;
}
std::set<unsigned> nums;
auto r = 0ULL;
while (--m) {
std::cin >> x;
if (!std::cin || x < 1 || x > n) {
std::cerr << "Bad Input!n";
return EXIT_FAILURE;
}
auto [iter, created] = nums.insert(x);
if (created)
r += 2 * x;
std::cout << r << 'n';
}
}
$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
8 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
8 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
7 hours ago
$begingroup$
I disagree that omitting thereturn
is "common" in C++. Sure, it happens, and it's something to be aware of, but I certainly wouldn't think anything of an interview candidate who included it. I write it myself, and I think it's excellent for clarity. Personally, though, I preferreturn EXIT_SUCCESS
if that's what you actually mean (or, conversely,return EXIT_FAILURE
) from<cstdlib>
.
$endgroup$
– Cody Gray
32 mins 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() {
unsigned n, m, x;
std::cin >> n >> m;
if (!std::cin || !n) {
std::cerr << "Bad Input!n";
return EXIT_FAILURE;
}
std::set<unsigned> nums;
auto r = 0ULL;
while (--m) {
std::cin >> x;
if (!std::cin || x < 1 || x > n) {
std::cerr << "Bad Input!n";
return EXIT_FAILURE;
}
auto [iter, created] = nums.insert(x);
if (created)
r += 2 * x;
std::cout << r << 'n';
}
}
$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() {
unsigned n, m, x;
std::cin >> n >> m;
if (!std::cin || !n) {
std::cerr << "Bad Input!n";
return EXIT_FAILURE;
}
std::set<unsigned> nums;
auto r = 0ULL;
while (--m) {
std::cin >> x;
if (!std::cin || x < 1 || x > n) {
std::cerr << "Bad Input!n";
return EXIT_FAILURE;
}
auto [iter, created] = nums.insert(x);
if (created)
r += 2 * x;
std::cout << r << 'n';
}
}
$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() {
unsigned n, m, x;
std::cin >> n >> m;
if (!std::cin || !n) {
std::cerr << "Bad Input!n";
return EXIT_FAILURE;
}
std::set<unsigned> nums;
auto r = 0ULL;
while (--m) {
std::cin >> x;
if (!std::cin || x < 1 || x > n) {
std::cerr << "Bad Input!n";
return EXIT_FAILURE;
}
auto [iter, created] = nums.insert(x);
if (created)
r += 2 * x;
std::cout << r << 'n';
}
}
$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() {
unsigned n, m, x;
std::cin >> n >> m;
if (!std::cin || !n) {
std::cerr << "Bad Input!n";
return EXIT_FAILURE;
}
std::set<unsigned> nums;
auto r = 0ULL;
while (--m) {
std::cin >> x;
if (!std::cin || x < 1 || x > n) {
std::cerr << "Bad Input!n";
return EXIT_FAILURE;
}
auto [iter, created] = nums.insert(x);
if (created)
r += 2 * x;
std::cout << r << 'n';
}
}
edited 5 hours ago
answered 7 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
8 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
8 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
7 hours ago
$begingroup$
I disagree that omitting thereturn
is "common" in C++. Sure, it happens, and it's something to be aware of, but I certainly wouldn't think anything of an interview candidate who included it. I write it myself, and I think it's excellent for clarity. Personally, though, I preferreturn EXIT_SUCCESS
if that's what you actually mean (or, conversely,return EXIT_FAILURE
) from<cstdlib>
.
$endgroup$
– Cody Gray
32 mins 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
8 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
8 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
7 hours ago
$begingroup$
I disagree that omitting thereturn
is "common" in C++. Sure, it happens, and it's something to be aware of, but I certainly wouldn't think anything of an interview candidate who included it. I write it myself, and I think it's excellent for clarity. Personally, though, I preferreturn EXIT_SUCCESS
if that's what you actually mean (or, conversely,return EXIT_FAILURE
) from<cstdlib>
.
$endgroup$
– Cody Gray
32 mins 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 7 hours ago
Jerry Coffin
28.9k462130
28.9k462130
answered 8 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
8 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
8 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
7 hours ago
$begingroup$
I disagree that omitting thereturn
is "common" in C++. Sure, it happens, and it's something to be aware of, but I certainly wouldn't think anything of an interview candidate who included it. I write it myself, and I think it's excellent for clarity. Personally, though, I preferreturn EXIT_SUCCESS
if that's what you actually mean (or, conversely,return EXIT_FAILURE
) from<cstdlib>
.
$endgroup$
– Cody Gray
32 mins 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
8 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
8 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
7 hours ago
$begingroup$
I disagree that omitting thereturn
is "common" in C++. Sure, it happens, and it's something to be aware of, but I certainly wouldn't think anything of an interview candidate who included it. I write it myself, and I think it's excellent for clarity. Personally, though, I preferreturn EXIT_SUCCESS
if that's what you actually mean (or, conversely,return EXIT_FAILURE
) from<cstdlib>
.
$endgroup$
– Cody Gray
32 mins 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
8 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
8 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
8 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
8 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
7 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
7 hours ago
$begingroup$
I disagree that omitting the
return
is "common" in C++. Sure, it happens, and it's something to be aware of, but I certainly wouldn't think anything of an interview candidate who included it. I write it myself, and I think it's excellent for clarity. Personally, though, I prefer return EXIT_SUCCESS
if that's what you actually mean (or, conversely, return EXIT_FAILURE
) from <cstdlib>
.$endgroup$
– Cody Gray
32 mins ago
$begingroup$
I disagree that omitting the
return
is "common" in C++. Sure, it happens, and it's something to be aware of, but I certainly wouldn't think anything of an interview candidate who included it. I write it myself, and I think it's excellent for clarity. Personally, though, I prefer return EXIT_SUCCESS
if that's what you actually mean (or, conversely, return EXIT_FAILURE
) from <cstdlib>
.$endgroup$
– Cody Gray
32 mins 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