Finding index in list in C++
$begingroup$
I need to find correct index in a array by comparing ranges. Here is my first try:
int TunerDriver::TunerLookUpTable(double Freq) {
Freq /= 1e6;
if (Freq >= 60 and Freq < 140)
return 1;
else if (Freq >= 180 and Freq < 280)
return 2;
else if (Freq >= 280 and Freq < 460)
return 3;
else if (Freq >= 140 and Freq < 180)
return 4;
else if (Freq >= 2720 and Freq < 4020)
return 5;
else if (Freq >= 4020 and Freq <= 6000)
return 6;
else if (Freq >= 2000 and Freq < 2720)
return 7;
else if (Freq >= 830 and Freq < 1420)
return 8;
else if (Freq >= 1420 and Freq < 2000)
return 9;
else if (Freq >= 460 and Freq < 830)
return 10;
else {
//TODO: throw exception
}
}
I believe I can use std::pair
and for
to increase readability. Can you assist me in this case?
c++ c++11
New contributor
$endgroup$
add a comment |
$begingroup$
I need to find correct index in a array by comparing ranges. Here is my first try:
int TunerDriver::TunerLookUpTable(double Freq) {
Freq /= 1e6;
if (Freq >= 60 and Freq < 140)
return 1;
else if (Freq >= 180 and Freq < 280)
return 2;
else if (Freq >= 280 and Freq < 460)
return 3;
else if (Freq >= 140 and Freq < 180)
return 4;
else if (Freq >= 2720 and Freq < 4020)
return 5;
else if (Freq >= 4020 and Freq <= 6000)
return 6;
else if (Freq >= 2000 and Freq < 2720)
return 7;
else if (Freq >= 830 and Freq < 1420)
return 8;
else if (Freq >= 1420 and Freq < 2000)
return 9;
else if (Freq >= 460 and Freq < 830)
return 10;
else {
//TODO: throw exception
}
}
I believe I can use std::pair
and for
to increase readability. Can you assist me in this case?
c++ c++11
New contributor
$endgroup$
$begingroup$
You could more simply usestd::map()
and/orstd::lower_bound()
. But you really need to improve the question - why do you need to do this lookup? What do the return values actually mean?
$endgroup$
– Toby Speight
15 hours ago
add a comment |
$begingroup$
I need to find correct index in a array by comparing ranges. Here is my first try:
int TunerDriver::TunerLookUpTable(double Freq) {
Freq /= 1e6;
if (Freq >= 60 and Freq < 140)
return 1;
else if (Freq >= 180 and Freq < 280)
return 2;
else if (Freq >= 280 and Freq < 460)
return 3;
else if (Freq >= 140 and Freq < 180)
return 4;
else if (Freq >= 2720 and Freq < 4020)
return 5;
else if (Freq >= 4020 and Freq <= 6000)
return 6;
else if (Freq >= 2000 and Freq < 2720)
return 7;
else if (Freq >= 830 and Freq < 1420)
return 8;
else if (Freq >= 1420 and Freq < 2000)
return 9;
else if (Freq >= 460 and Freq < 830)
return 10;
else {
//TODO: throw exception
}
}
I believe I can use std::pair
and for
to increase readability. Can you assist me in this case?
c++ c++11
New contributor
$endgroup$
I need to find correct index in a array by comparing ranges. Here is my first try:
int TunerDriver::TunerLookUpTable(double Freq) {
Freq /= 1e6;
if (Freq >= 60 and Freq < 140)
return 1;
else if (Freq >= 180 and Freq < 280)
return 2;
else if (Freq >= 280 and Freq < 460)
return 3;
else if (Freq >= 140 and Freq < 180)
return 4;
else if (Freq >= 2720 and Freq < 4020)
return 5;
else if (Freq >= 4020 and Freq <= 6000)
return 6;
else if (Freq >= 2000 and Freq < 2720)
return 7;
else if (Freq >= 830 and Freq < 1420)
return 8;
else if (Freq >= 1420 and Freq < 2000)
return 9;
else if (Freq >= 460 and Freq < 830)
return 10;
else {
//TODO: throw exception
}
}
I believe I can use std::pair
and for
to increase readability. Can you assist me in this case?
c++ c++11
c++ c++11
New contributor
New contributor
edited 2 hours ago
Jamal♦
30.4k11121227
30.4k11121227
New contributor
asked 17 hours ago
Ali NikneshanAli Nikneshan
1041
1041
New contributor
New contributor
$begingroup$
You could more simply usestd::map()
and/orstd::lower_bound()
. But you really need to improve the question - why do you need to do this lookup? What do the return values actually mean?
$endgroup$
– Toby Speight
15 hours ago
add a comment |
$begingroup$
You could more simply usestd::map()
and/orstd::lower_bound()
. But you really need to improve the question - why do you need to do this lookup? What do the return values actually mean?
$endgroup$
– Toby Speight
15 hours ago
$begingroup$
You could more simply use
std::map()
and/or std::lower_bound()
. But you really need to improve the question - why do you need to do this lookup? What do the return values actually mean?$endgroup$
– Toby Speight
15 hours ago
$begingroup$
You could more simply use
std::map()
and/or std::lower_bound()
. But you really need to improve the question - why do you need to do this lookup? What do the return values actually mean?$endgroup$
– Toby Speight
15 hours ago
add a comment |
1 Answer
1
active
oldest
votes
$begingroup$
It's a short bit of code, but enough to work with, I think. Here are some suggestions to help you improve your code.
Minimize the number of comparisons
There is not really a need to check both upper and lower bounds for each possibility if the comparisons are done in order. That is, we could compare the incoming Freq
to 60, then 140, then 180, etc. That might look like this:
int FreqLookup(double Freq) {
if (Freq < 60e6 || Freq > 6000e6) {
return 0; // or throw an error
}
if (Freq < 140e6) return 1;
if (Freq < 180e6) return 4;
if (Freq < 280e6) return 2;
if (Freq < 460e6) return 3;
if (Freq < 830e6) return 10;
if (Freq < 1420e6) return 8;
if (Freq < 2000e6) return 9;
if (Freq < 2720e6) return 7;
if (Freq < 4020e6) return 5;
return 6;
}
Put constants in a structure
Having the constants in a structure allows the code to be more data driven. Here's what I'd suggest:
struct FreqTableEntry {
double freq;
int divisor;
operator double() const { return freq; }
};
static constexpr std::array<FreqTableEntry, 11> lookup {{
{ 60e6, 0 }, // error if below 60e6
{ 140e6, 1 },
{ 180e6, 4 },
{ 280e6, 2 },
{ 460e6, 3 },
{ 830e6, 10 },
{ 1420e6, 8 },
{ 2000e6, 9 },
{ 2720e6, 7 },
{ 4020e6, 5 },
{ 6000e6, 6 },
}};
Now we have a nice, neat, compile-time lookup table. Note also that we can avoid division by simply using the full value in the table.
Use a standard algorithm
The std::upper_bound
algorithm returns an iterator to the first entry that is greater than the given value. We can use it:
auto it{std::upper_bound(lookup.cbegin(), lookup.cend(), Freq)};
It would be nice to simply return it->divisor
but we have to handle a few special cases. First, if the frequency is exactly 6000e6, we need to return 6. Next, if the frequency is < 60e6 or > 6000e6, we need to indicate an error. I've chosen to return 0 as an indication of error, but one could also throw
if that's more appropriate. Putting it all together we have this:
#include <array>
#include <algorithm>
int alt(double Freq) {
struct FreqTableEntry {
double freq;
int divisor;
operator double() const { return freq; }
};
static constexpr std::array<FreqTableEntry, 11> lookup {{
{ 60e6, 0 }, // error if below 60e6
{ 140e6, 1 },
{ 180e6, 4 },
{ 280e6, 2 },
{ 460e6, 3 },
{ 830e6, 10 },
{ 1420e6, 8 },
{ 2000e6, 9 },
{ 2720e6, 7 },
{ 4020e6, 5 },
{ 6000e6, 6 },
}};
// special case the last entry
if (Freq == lookup.back().freq) {
return lookup.back().divisor;
}
auto it{std::upper_bound(lookup.cbegin(), lookup.cend(), Freq)};
if (it == lookup.cend() || it->divisor == 0) {
return 0; // could throw here as alternative
}
return it->divisor;
}
Provide a test program
It's often helpful to write a test program and to provide it to reviewers to make it clear what the program is expected to do and how it will be called. I modified your routine slightly to return 0 as an error indication and made it a standalone function. The test program in this case, was just to compare the original routine to the alternative (named alt
) shown above:
#include <iostream>
int main() {
for (double f{30e6}; f <= 6020e6; f += 10e6) {
if (TunerLookUpTable(f) != alt(f)) {
std::cout << "ERROR: TunerLookUpTable(" << f/1e6 << " MHz) = "
<< TunerLookUpTable(f) << ", but alt(f) = " << alt(f) << 'n';
}
}
}
$endgroup$
$begingroup$
The first code block differs from the original if passed a NaN (returns 6 rather than 0 or throw). The subsequent examples look okay.
$endgroup$
– Toby Speight
11 hours ago
$begingroup$
That would be a useful condition to add to a test program.
$endgroup$
– Edward
11 hours ago
add a comment |
Your Answer
StackExchange.ifUsing("editor", function () {
return StackExchange.using("mathjaxEditing", function () {
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
});
});
}, "mathjax-editing");
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
});
}
});
Ali Nikneshan is a new contributor. Be nice, and check out our Code of Conduct.
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%2f215251%2ffinding-index-in-list-in-c%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
1 Answer
1
active
oldest
votes
1 Answer
1
active
oldest
votes
active
oldest
votes
active
oldest
votes
$begingroup$
It's a short bit of code, but enough to work with, I think. Here are some suggestions to help you improve your code.
Minimize the number of comparisons
There is not really a need to check both upper and lower bounds for each possibility if the comparisons are done in order. That is, we could compare the incoming Freq
to 60, then 140, then 180, etc. That might look like this:
int FreqLookup(double Freq) {
if (Freq < 60e6 || Freq > 6000e6) {
return 0; // or throw an error
}
if (Freq < 140e6) return 1;
if (Freq < 180e6) return 4;
if (Freq < 280e6) return 2;
if (Freq < 460e6) return 3;
if (Freq < 830e6) return 10;
if (Freq < 1420e6) return 8;
if (Freq < 2000e6) return 9;
if (Freq < 2720e6) return 7;
if (Freq < 4020e6) return 5;
return 6;
}
Put constants in a structure
Having the constants in a structure allows the code to be more data driven. Here's what I'd suggest:
struct FreqTableEntry {
double freq;
int divisor;
operator double() const { return freq; }
};
static constexpr std::array<FreqTableEntry, 11> lookup {{
{ 60e6, 0 }, // error if below 60e6
{ 140e6, 1 },
{ 180e6, 4 },
{ 280e6, 2 },
{ 460e6, 3 },
{ 830e6, 10 },
{ 1420e6, 8 },
{ 2000e6, 9 },
{ 2720e6, 7 },
{ 4020e6, 5 },
{ 6000e6, 6 },
}};
Now we have a nice, neat, compile-time lookup table. Note also that we can avoid division by simply using the full value in the table.
Use a standard algorithm
The std::upper_bound
algorithm returns an iterator to the first entry that is greater than the given value. We can use it:
auto it{std::upper_bound(lookup.cbegin(), lookup.cend(), Freq)};
It would be nice to simply return it->divisor
but we have to handle a few special cases. First, if the frequency is exactly 6000e6, we need to return 6. Next, if the frequency is < 60e6 or > 6000e6, we need to indicate an error. I've chosen to return 0 as an indication of error, but one could also throw
if that's more appropriate. Putting it all together we have this:
#include <array>
#include <algorithm>
int alt(double Freq) {
struct FreqTableEntry {
double freq;
int divisor;
operator double() const { return freq; }
};
static constexpr std::array<FreqTableEntry, 11> lookup {{
{ 60e6, 0 }, // error if below 60e6
{ 140e6, 1 },
{ 180e6, 4 },
{ 280e6, 2 },
{ 460e6, 3 },
{ 830e6, 10 },
{ 1420e6, 8 },
{ 2000e6, 9 },
{ 2720e6, 7 },
{ 4020e6, 5 },
{ 6000e6, 6 },
}};
// special case the last entry
if (Freq == lookup.back().freq) {
return lookup.back().divisor;
}
auto it{std::upper_bound(lookup.cbegin(), lookup.cend(), Freq)};
if (it == lookup.cend() || it->divisor == 0) {
return 0; // could throw here as alternative
}
return it->divisor;
}
Provide a test program
It's often helpful to write a test program and to provide it to reviewers to make it clear what the program is expected to do and how it will be called. I modified your routine slightly to return 0 as an error indication and made it a standalone function. The test program in this case, was just to compare the original routine to the alternative (named alt
) shown above:
#include <iostream>
int main() {
for (double f{30e6}; f <= 6020e6; f += 10e6) {
if (TunerLookUpTable(f) != alt(f)) {
std::cout << "ERROR: TunerLookUpTable(" << f/1e6 << " MHz) = "
<< TunerLookUpTable(f) << ", but alt(f) = " << alt(f) << 'n';
}
}
}
$endgroup$
$begingroup$
The first code block differs from the original if passed a NaN (returns 6 rather than 0 or throw). The subsequent examples look okay.
$endgroup$
– Toby Speight
11 hours ago
$begingroup$
That would be a useful condition to add to a test program.
$endgroup$
– Edward
11 hours ago
add a comment |
$begingroup$
It's a short bit of code, but enough to work with, I think. Here are some suggestions to help you improve your code.
Minimize the number of comparisons
There is not really a need to check both upper and lower bounds for each possibility if the comparisons are done in order. That is, we could compare the incoming Freq
to 60, then 140, then 180, etc. That might look like this:
int FreqLookup(double Freq) {
if (Freq < 60e6 || Freq > 6000e6) {
return 0; // or throw an error
}
if (Freq < 140e6) return 1;
if (Freq < 180e6) return 4;
if (Freq < 280e6) return 2;
if (Freq < 460e6) return 3;
if (Freq < 830e6) return 10;
if (Freq < 1420e6) return 8;
if (Freq < 2000e6) return 9;
if (Freq < 2720e6) return 7;
if (Freq < 4020e6) return 5;
return 6;
}
Put constants in a structure
Having the constants in a structure allows the code to be more data driven. Here's what I'd suggest:
struct FreqTableEntry {
double freq;
int divisor;
operator double() const { return freq; }
};
static constexpr std::array<FreqTableEntry, 11> lookup {{
{ 60e6, 0 }, // error if below 60e6
{ 140e6, 1 },
{ 180e6, 4 },
{ 280e6, 2 },
{ 460e6, 3 },
{ 830e6, 10 },
{ 1420e6, 8 },
{ 2000e6, 9 },
{ 2720e6, 7 },
{ 4020e6, 5 },
{ 6000e6, 6 },
}};
Now we have a nice, neat, compile-time lookup table. Note also that we can avoid division by simply using the full value in the table.
Use a standard algorithm
The std::upper_bound
algorithm returns an iterator to the first entry that is greater than the given value. We can use it:
auto it{std::upper_bound(lookup.cbegin(), lookup.cend(), Freq)};
It would be nice to simply return it->divisor
but we have to handle a few special cases. First, if the frequency is exactly 6000e6, we need to return 6. Next, if the frequency is < 60e6 or > 6000e6, we need to indicate an error. I've chosen to return 0 as an indication of error, but one could also throw
if that's more appropriate. Putting it all together we have this:
#include <array>
#include <algorithm>
int alt(double Freq) {
struct FreqTableEntry {
double freq;
int divisor;
operator double() const { return freq; }
};
static constexpr std::array<FreqTableEntry, 11> lookup {{
{ 60e6, 0 }, // error if below 60e6
{ 140e6, 1 },
{ 180e6, 4 },
{ 280e6, 2 },
{ 460e6, 3 },
{ 830e6, 10 },
{ 1420e6, 8 },
{ 2000e6, 9 },
{ 2720e6, 7 },
{ 4020e6, 5 },
{ 6000e6, 6 },
}};
// special case the last entry
if (Freq == lookup.back().freq) {
return lookup.back().divisor;
}
auto it{std::upper_bound(lookup.cbegin(), lookup.cend(), Freq)};
if (it == lookup.cend() || it->divisor == 0) {
return 0; // could throw here as alternative
}
return it->divisor;
}
Provide a test program
It's often helpful to write a test program and to provide it to reviewers to make it clear what the program is expected to do and how it will be called. I modified your routine slightly to return 0 as an error indication and made it a standalone function. The test program in this case, was just to compare the original routine to the alternative (named alt
) shown above:
#include <iostream>
int main() {
for (double f{30e6}; f <= 6020e6; f += 10e6) {
if (TunerLookUpTable(f) != alt(f)) {
std::cout << "ERROR: TunerLookUpTable(" << f/1e6 << " MHz) = "
<< TunerLookUpTable(f) << ", but alt(f) = " << alt(f) << 'n';
}
}
}
$endgroup$
$begingroup$
The first code block differs from the original if passed a NaN (returns 6 rather than 0 or throw). The subsequent examples look okay.
$endgroup$
– Toby Speight
11 hours ago
$begingroup$
That would be a useful condition to add to a test program.
$endgroup$
– Edward
11 hours ago
add a comment |
$begingroup$
It's a short bit of code, but enough to work with, I think. Here are some suggestions to help you improve your code.
Minimize the number of comparisons
There is not really a need to check both upper and lower bounds for each possibility if the comparisons are done in order. That is, we could compare the incoming Freq
to 60, then 140, then 180, etc. That might look like this:
int FreqLookup(double Freq) {
if (Freq < 60e6 || Freq > 6000e6) {
return 0; // or throw an error
}
if (Freq < 140e6) return 1;
if (Freq < 180e6) return 4;
if (Freq < 280e6) return 2;
if (Freq < 460e6) return 3;
if (Freq < 830e6) return 10;
if (Freq < 1420e6) return 8;
if (Freq < 2000e6) return 9;
if (Freq < 2720e6) return 7;
if (Freq < 4020e6) return 5;
return 6;
}
Put constants in a structure
Having the constants in a structure allows the code to be more data driven. Here's what I'd suggest:
struct FreqTableEntry {
double freq;
int divisor;
operator double() const { return freq; }
};
static constexpr std::array<FreqTableEntry, 11> lookup {{
{ 60e6, 0 }, // error if below 60e6
{ 140e6, 1 },
{ 180e6, 4 },
{ 280e6, 2 },
{ 460e6, 3 },
{ 830e6, 10 },
{ 1420e6, 8 },
{ 2000e6, 9 },
{ 2720e6, 7 },
{ 4020e6, 5 },
{ 6000e6, 6 },
}};
Now we have a nice, neat, compile-time lookup table. Note also that we can avoid division by simply using the full value in the table.
Use a standard algorithm
The std::upper_bound
algorithm returns an iterator to the first entry that is greater than the given value. We can use it:
auto it{std::upper_bound(lookup.cbegin(), lookup.cend(), Freq)};
It would be nice to simply return it->divisor
but we have to handle a few special cases. First, if the frequency is exactly 6000e6, we need to return 6. Next, if the frequency is < 60e6 or > 6000e6, we need to indicate an error. I've chosen to return 0 as an indication of error, but one could also throw
if that's more appropriate. Putting it all together we have this:
#include <array>
#include <algorithm>
int alt(double Freq) {
struct FreqTableEntry {
double freq;
int divisor;
operator double() const { return freq; }
};
static constexpr std::array<FreqTableEntry, 11> lookup {{
{ 60e6, 0 }, // error if below 60e6
{ 140e6, 1 },
{ 180e6, 4 },
{ 280e6, 2 },
{ 460e6, 3 },
{ 830e6, 10 },
{ 1420e6, 8 },
{ 2000e6, 9 },
{ 2720e6, 7 },
{ 4020e6, 5 },
{ 6000e6, 6 },
}};
// special case the last entry
if (Freq == lookup.back().freq) {
return lookup.back().divisor;
}
auto it{std::upper_bound(lookup.cbegin(), lookup.cend(), Freq)};
if (it == lookup.cend() || it->divisor == 0) {
return 0; // could throw here as alternative
}
return it->divisor;
}
Provide a test program
It's often helpful to write a test program and to provide it to reviewers to make it clear what the program is expected to do and how it will be called. I modified your routine slightly to return 0 as an error indication and made it a standalone function. The test program in this case, was just to compare the original routine to the alternative (named alt
) shown above:
#include <iostream>
int main() {
for (double f{30e6}; f <= 6020e6; f += 10e6) {
if (TunerLookUpTable(f) != alt(f)) {
std::cout << "ERROR: TunerLookUpTable(" << f/1e6 << " MHz) = "
<< TunerLookUpTable(f) << ", but alt(f) = " << alt(f) << 'n';
}
}
}
$endgroup$
It's a short bit of code, but enough to work with, I think. Here are some suggestions to help you improve your code.
Minimize the number of comparisons
There is not really a need to check both upper and lower bounds for each possibility if the comparisons are done in order. That is, we could compare the incoming Freq
to 60, then 140, then 180, etc. That might look like this:
int FreqLookup(double Freq) {
if (Freq < 60e6 || Freq > 6000e6) {
return 0; // or throw an error
}
if (Freq < 140e6) return 1;
if (Freq < 180e6) return 4;
if (Freq < 280e6) return 2;
if (Freq < 460e6) return 3;
if (Freq < 830e6) return 10;
if (Freq < 1420e6) return 8;
if (Freq < 2000e6) return 9;
if (Freq < 2720e6) return 7;
if (Freq < 4020e6) return 5;
return 6;
}
Put constants in a structure
Having the constants in a structure allows the code to be more data driven. Here's what I'd suggest:
struct FreqTableEntry {
double freq;
int divisor;
operator double() const { return freq; }
};
static constexpr std::array<FreqTableEntry, 11> lookup {{
{ 60e6, 0 }, // error if below 60e6
{ 140e6, 1 },
{ 180e6, 4 },
{ 280e6, 2 },
{ 460e6, 3 },
{ 830e6, 10 },
{ 1420e6, 8 },
{ 2000e6, 9 },
{ 2720e6, 7 },
{ 4020e6, 5 },
{ 6000e6, 6 },
}};
Now we have a nice, neat, compile-time lookup table. Note also that we can avoid division by simply using the full value in the table.
Use a standard algorithm
The std::upper_bound
algorithm returns an iterator to the first entry that is greater than the given value. We can use it:
auto it{std::upper_bound(lookup.cbegin(), lookup.cend(), Freq)};
It would be nice to simply return it->divisor
but we have to handle a few special cases. First, if the frequency is exactly 6000e6, we need to return 6. Next, if the frequency is < 60e6 or > 6000e6, we need to indicate an error. I've chosen to return 0 as an indication of error, but one could also throw
if that's more appropriate. Putting it all together we have this:
#include <array>
#include <algorithm>
int alt(double Freq) {
struct FreqTableEntry {
double freq;
int divisor;
operator double() const { return freq; }
};
static constexpr std::array<FreqTableEntry, 11> lookup {{
{ 60e6, 0 }, // error if below 60e6
{ 140e6, 1 },
{ 180e6, 4 },
{ 280e6, 2 },
{ 460e6, 3 },
{ 830e6, 10 },
{ 1420e6, 8 },
{ 2000e6, 9 },
{ 2720e6, 7 },
{ 4020e6, 5 },
{ 6000e6, 6 },
}};
// special case the last entry
if (Freq == lookup.back().freq) {
return lookup.back().divisor;
}
auto it{std::upper_bound(lookup.cbegin(), lookup.cend(), Freq)};
if (it == lookup.cend() || it->divisor == 0) {
return 0; // could throw here as alternative
}
return it->divisor;
}
Provide a test program
It's often helpful to write a test program and to provide it to reviewers to make it clear what the program is expected to do and how it will be called. I modified your routine slightly to return 0 as an error indication and made it a standalone function. The test program in this case, was just to compare the original routine to the alternative (named alt
) shown above:
#include <iostream>
int main() {
for (double f{30e6}; f <= 6020e6; f += 10e6) {
if (TunerLookUpTable(f) != alt(f)) {
std::cout << "ERROR: TunerLookUpTable(" << f/1e6 << " MHz) = "
<< TunerLookUpTable(f) << ", but alt(f) = " << alt(f) << 'n';
}
}
}
edited 15 hours ago
answered 15 hours ago
EdwardEdward
47.2k378212
47.2k378212
$begingroup$
The first code block differs from the original if passed a NaN (returns 6 rather than 0 or throw). The subsequent examples look okay.
$endgroup$
– Toby Speight
11 hours ago
$begingroup$
That would be a useful condition to add to a test program.
$endgroup$
– Edward
11 hours ago
add a comment |
$begingroup$
The first code block differs from the original if passed a NaN (returns 6 rather than 0 or throw). The subsequent examples look okay.
$endgroup$
– Toby Speight
11 hours ago
$begingroup$
That would be a useful condition to add to a test program.
$endgroup$
– Edward
11 hours ago
$begingroup$
The first code block differs from the original if passed a NaN (returns 6 rather than 0 or throw). The subsequent examples look okay.
$endgroup$
– Toby Speight
11 hours ago
$begingroup$
The first code block differs from the original if passed a NaN (returns 6 rather than 0 or throw). The subsequent examples look okay.
$endgroup$
– Toby Speight
11 hours ago
$begingroup$
That would be a useful condition to add to a test program.
$endgroup$
– Edward
11 hours ago
$begingroup$
That would be a useful condition to add to a test program.
$endgroup$
– Edward
11 hours ago
add a comment |
Ali Nikneshan is a new contributor. Be nice, and check out our Code of Conduct.
Ali Nikneshan is a new contributor. Be nice, and check out our Code of Conduct.
Ali Nikneshan is a new contributor. Be nice, and check out our Code of Conduct.
Ali Nikneshan is a new contributor. Be nice, and check out our Code of Conduct.
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%2f215251%2ffinding-index-in-list-in-c%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
$begingroup$
You could more simply use
std::map()
and/orstd::lower_bound()
. But you really need to improve the question - why do you need to do this lookup? What do the return values actually mean?$endgroup$
– Toby Speight
15 hours ago