HackerRank Lower Bound-STL

Question

This is still my first week of learning C++, I solved the lower bound STL challenge in HackerRank Lower Bound STL but I'm having time exceeded issue for long input. I'm open to constructive criticism, ways to improve in terms of syntax, style and performance.

You are given \$N\$ integers in the sorted order. Then you are given \$Q\$ queries. In each query, you will be given an integer and you have to tell whether that integer is present in the array if so you have to tell at which index it is present and if it is not present you have to tell the index at which the smallest integer that is just greater than the given number is present. Lower bound is a function that can be used with a sorted vector.

Input Format

The first line of the input contains the number of integers \$N\$. The next line contains \$N\$ integers in sorted order. The next line contains \$Q\$, the number of queries. Then \$Q\$ lines follow each containing a single integer \$Y\$.
If the same number is present multiple times, you have to print the first index at which it occurs. The input is such that you always have an answer for each query.

Constraints

• \1ドル \leq N \leq 10^5\$
• \1ドル \leq X_i \leq 10^9\$, where \$X_i\$ is \$i^{th}\$ element in the array
• \1ドル \leq Q \leq 10^5\$
• \1ドル \leq Y \leq 10^9\$

Output Format

For each query you have to print "Yes"(without the quotes) if the number is present and at which index it is present separated by a space. If the number is not present you have to print "No"(without the quotes) followed by the index of the next smallest number just greater than that number. You have to output each query in a new line.

Sample Input

 8
 1 1 2 2 6 9 9 15
 4
 1
 4
 9
 15

Sample Output

 Yes 1
 No 5
 Yes 6
 Yes 8

Here is my code

#include <cmath>
#include <cstdio>
#include <vector>
#include <iostream>
#include <algorithm>
int main() {
 /* Enter your code here. Read input from STDIN. Print output to STDOUT */ 
 int N;
 int query;
 int queryLength;
 {
 
 std::vector<int>::iterator low;
 std::vector<int>::iterator foundValue;
 std::cin >> N;
 
 std::vector<int> v(N);
 //populate the vector
 for(int i =0; i<N; i++){
 std::cin >> v[i];
 }
 
 std::cin >> queryLength;
 
 for(int j = 0; j < queryLength; j++){
 std::cin >> query;
 foundValue= find(v.begin(), v.end(), query);
 if (foundValue != v.end())
 {
 std::cout << "Yes "<< (foundValue-v.begin()+ 1) << std::endl;
 }
 else{
 low = std::lower_bound (v.begin(), v.end(),query);
 std::cout << "No "<< find(v.begin(), v.end(), *low)-v.begin()+ 1 << std::endl;
 }
 }
}
 return 0;
}
 

• Loki Astari: Why use find and lower_bound. find() is O(n) while lower_bound() is O(ln(n)). You can use std::distance() to find the index from two iterators.
• CiaPan: I am pretty sure the values looking like 105 and 109, given as constraints, should be the fith and the ninth power of 10.
• CiaPan: Suppose the N in 'Then N lines follow' should read Q...

Answer 1

Although you've probably already solved this problem, I'll still post my review for completion's sake.

1. Performance

Just as Loki said, you should use std::lower_bound() and std::distance() to solve this task, because std::find() has higher complexity.

2. General tips

1. Do not include unused libraries (in this case, <cmath> and <cstdio>).

2. Only declare variables when you need them. Declaring all the variables at the beginning of the program is old C style practice, which is discouraged in C++.

3. Prefer prefix to postfix operator++. The postfix version creates a copy, so if you do not need that copy, you should use the prefix version.

4. If you have access to C++11, you should consider using auto more. It'll make your code easier to read and save you some typing.

5. You can safely omit return 0 from main();

[C++11: 3.6.1/5]: A return statement in main has the effect of leaving the main function (destroying any objects with automatic storage duration) and calling std::exit with the return value as the argument. If control reaches the end of main without encountering a return statement, the effect is that of executing return 0;.

6. Give your variables better names. Although other variable names in your code are good, a vector named v is not very descriptive.

7. Use const_iterators for functions that do not alter the original range.

3. Final code

#include <vector>
#include <iostream>
#include <algorithm>
int main() {
 int N;
 std::cin >> N;
 
 std::vector<int> numbers(N);
 //populate the vector
 for(int i = 0; i < N; ++i) {
 std::cin >> numbers[i];
 }
 
 int queryLength;
 std::cin >> queryLength; 
 for(int j = 0; j < queryLength; ++j) {
 int query;
 std::cin >> query;
 auto lowerBoundIt = std::lower_bound(numbers.cbegin(), numbers.cend(), query);
 if(*lowerBoundIt == query) {
 std::cout << "Yes ";
 }
 else {
 std::cout << "No ";
 }
 std::cout << std::distance(numbers.cbegin(), lowerBoundIt) + 1 << '\n';
 }
}

• Toby Speight: A further improvement could be to use the previously-found result to narrow the search to one side or the other - the gain is probably not worth the complexity, though.

Answer 2

You've created an unnecessary scope by putting braces after int querylength. Do you have a good reason for doing this?