#include <iostream>
#include <vector>
#include <algorithm>
In smaller programs like yours, traversing a list of 3 #include
s isn't an issue. In larger programs, the list of #include
d libraries can become large. Organizing your #include
s into logical groups and alphabetizing those subgroups helps determine if something is #include
d. Think about how you search a phone book. You typically use some pivoted search to narrow down your search range. The same applies when looking at a list of library #include
s.
sort(v.begin(), v.end());
Unless your intent is to invoke argument dependent lookup (ADL), I would suggest you qualify your calls. As the reader, I'm left to try to figure out if this is a call to std::sort
or your own rolled sort
. ADL can also result in an unintended function being called an unintended function being called.
If your intent was to use ADL, be explicit with a using
-declaration.
using std::swap; // Explicit: opting into ADL.
swap(lhs, rhs); // unqualified lookup, find the best match.
for (int i = 1, countMode = 1, count = 1; i < v.size(); ++i) {
if (v[i] == number)
++countMode;
if (countMode > count) {
count = countMode;
mode = number;
}
}
It should be noted that the calculation for the mode
is incorrect. You never reset the countMode
value, so your function returns the last duplicate value seen rather than the mode
.
Assuming you fix that, consider other possible sequence structures.
- Many duplicates - where
mode([1, 1, 2, 2, 3, 3])
should result in the multimodal set[1, 2, 3]
. - Unique values - where
mode([1, 2, 3])
should result in the empty set[]
. - Empty set - where
mode([])
should result in the empty set[]
.
Consider how your program handles these situations.
In the case of an empty set, v[0]
doesn't exist. Attempting to access v[0]
produces a segmentation fault. mode
is also never initialized and your program will exhibit undefined behavior if you try to do anything with that value.
While it is easy for beginner's to just dump everything into main()
, I would suggest you break your program up into smaller, focused, and testable logical abstractions. min
and max
elements are as simple as calls to front()
and back()
on sorted lists. If you want those values for unsorted lists, the standard library provides std::min_element
, std::max_element
and std::minmax_element
in the <algorithm>
library. mode
should be abstracted into it's own function as well utilizing other abstractions to accomplish its task.
#include <iostream>
#include <vector>
#include <algorithm>
In smaller programs like yours, traversing a list of 3 #include
s isn't an issue. In larger programs, the list of #include
d libraries can become large. Organizing your #include
s into logical groups and alphabetizing those subgroups helps determine if something is #include
d. Think about how you search a phone book. You typically use some pivoted search to narrow down your search range. The same applies when looking at a list of library #include
s.
sort(v.begin(), v.end());
Unless your intent is to invoke argument dependent lookup (ADL), I would suggest you qualify your calls. As the reader, I'm left to try to figure out if this is a call to std::sort
or your own rolled sort
. ADL can also result in an unintended function being called.
If your intent was to use ADL, be explicit with a using
-declaration.
using std::swap; // Explicit: opting into ADL.
swap(lhs, rhs); // unqualified lookup, find the best match.
for (int i = 1, countMode = 1, count = 1; i < v.size(); ++i) {
if (v[i] == number)
++countMode;
if (countMode > count) {
count = countMode;
mode = number;
}
}
It should be noted that the calculation for the mode
is incorrect. You never reset the countMode
value, so your function returns the last duplicate value seen rather than the mode
.
Assuming you fix that, consider other possible sequence structures.
- Many duplicates - where
mode([1, 1, 2, 2, 3, 3])
should result in the multimodal set[1, 2, 3]
. - Unique values - where
mode([1, 2, 3])
should result in the empty set[]
. - Empty set - where
mode([])
should result in the empty set[]
.
Consider how your program handles these situations.
In the case of an empty set, v[0]
doesn't exist. Attempting to access v[0]
produces a segmentation fault. mode
is also never initialized and your program will exhibit undefined behavior if you try to do anything with that value.
While it is easy for beginner's to just dump everything into main()
, I would suggest you break your program up into smaller, focused, and testable logical abstractions. min
and max
elements are as simple as calls to front()
and back()
on sorted lists. If you want those values for unsorted lists, the standard library provides std::min_element
, std::max_element
and std::minmax_element
in the <algorithm>
library. mode
should be abstracted into it's own function as well utilizing other abstractions to accomplish its task.
#include <iostream>
#include <vector>
#include <algorithm>
In smaller programs like yours, traversing a list of 3 #include
s isn't an issue. In larger programs, the list of #include
d libraries can become large. Organizing your #include
s into logical groups and alphabetizing those subgroups helps determine if something is #include
d. Think about how you search a phone book. You typically use some pivoted search to narrow down your search range. The same applies when looking at a list of library #include
s.
sort(v.begin(), v.end());
Unless your intent is to invoke argument dependent lookup (ADL), I would suggest you qualify your calls. As the reader, I'm left to try to figure out if this is a call to std::sort
or your own rolled sort
. ADL can also result in an unintended function being called.
If your intent was to use ADL, be explicit with a using
-declaration.
using std::swap; // Explicit: opting into ADL.
swap(lhs, rhs); // unqualified lookup, find the best match.
for (int i = 1, countMode = 1, count = 1; i < v.size(); ++i) {
if (v[i] == number)
++countMode;
if (countMode > count) {
count = countMode;
mode = number;
}
}
It should be noted that the calculation for the mode
is incorrect. You never reset the countMode
value, so your function returns the last duplicate value seen rather than the mode
.
Assuming you fix that, consider other possible sequence structures.
- Many duplicates - where
mode([1, 1, 2, 2, 3, 3])
should result in the multimodal set[1, 2, 3]
. - Unique values - where
mode([1, 2, 3])
should result in the empty set[]
. - Empty set - where
mode([])
should result in the empty set[]
.
Consider how your program handles these situations.
In the case of an empty set, v[0]
doesn't exist. Attempting to access v[0]
produces a segmentation fault. mode
is also never initialized and your program will exhibit undefined behavior if you try to do anything with that value.
While it is easy for beginner's to just dump everything into main()
, I would suggest you break your program up into smaller, focused, and testable logical abstractions. min
and max
elements are as simple as calls to front()
and back()
on sorted lists. If you want those values for unsorted lists, the standard library provides std::min_element
, std::max_element
and std::minmax_element
in the <algorithm>
library. mode
should be abstracted into it's own function as well utilizing other abstractions to accomplish its task.
In the case of an empty set is processed, v[0]
exists turns out to be false as well with an empty sequencedoesn't exist. Attempting to access v[0]
in that case produces a segmentation fault. mode
is also never initialized and your program will exhibit undefined behavior if you try to do anything with that value.
In the case of an empty set is processed, v[0]
exists turns out to be false as well with an empty sequence. Attempting to access v[0]
in that case produces a segmentation fault. mode
is also never initialized and your program will exhibit undefined behavior if you try to do anything with that value.
In the case of an empty set, v[0]
doesn't exist. Attempting to access v[0]
produces a segmentation fault. mode
is also never initialized and your program will exhibit undefined behavior if you try to do anything with that value.
#include <iostream>
#include <vector>
#include <algorithm>
In smaller programs like yours, traversing a list of 3 #include
s isn't an issue. In larger programs, the list of #include
d libraries can become large. Organizing your #include
s into logical groups and alphabetizing those subgroups helps determine if something is #include
d. Think about how you search a phone book. You typically use some pivoted search to narrow down your search range. The same applies when looking at a list of library #include
s.
sort(v.begin(), v.end());
Unless your intent is to invoke argument dependent lookup (ADL), I would suggest you qualify your calls. As the reader, I'm left to try to figure out if this is a call to std::sort
or your own rolled sort
. ADL can also result in an unintended function being called.
If your intent was to use ADL, be explicit with a using
-declaration.
using std::swap; // Explicit: opting into ADL.
swap(lhs, rhs); // unqualified lookup, find the best match.
for (int i = 1, countMode = 1, count = 1; i < v.size(); ++i) {
if (v[i] == number)
++countMode;
if (countMode > count) {
count = countMode;
mode = number;
}
}
It should be noted that the calculation for the mode
is incorrect. You never reset the countMode
value, so your function returns the last duplicate value seen rather than the mode
.
Assuming you fix that, consider other possible sequence structures.
- Many duplicates - where
mode([1, 1, 2, 2, 3, 3])
should result in the multimodal set[1, 2, 3]
. - Unique values - where
mode([1, 2, 3])
should result in the empty set[]
. - Empty set - where
mode([])
should result in the empty set[]
.
Consider how your program handles these situations.
In the case of an empty set is processed, v[0]
exists turns out to be false as well with an empty sequence. Attempting to access v[0]
in that case produces a segmentation fault. mode
is also never initialized and your program will exhibit undefined behavior if you try to do anything with that value.
While it is easy for beginner's to just dump everything into main()
, I would suggest you break your program up into smaller, focused, and testable logical abstractions. min
and max
elements are as simple as calls to front()
and back()
on sorted lists. If you want those values for unsorted lists, the standard library provides std::min_element
, std::max_element
and std::minmax_element
in the <algorithm>
library. mode
should be abstracted into it's own function as well utilizing other abstractions to accomplish its task.