Ensure queue is in ascending order

Question

The program reads in a queue of integers and returns determines whether or not it is valid (ascending). How can this code be improved? I assume iterator include statement is unnecessary?

#include <iostream>
#include <queue>
#include <deque>
#include <iterator>
template<typename T, typename Container=std::deque<T> >
class iterable_queue : public std::queue<T,Container>
{
public:
 typedef typename Container::iterator iterator;
 typedef typename Container::const_iterator const_iterator;
 iterator begin() { return this->c.begin(); }
 iterator end() { return this->c.end(); }
 const_iterator begin() const { return this->c.begin(); }
 const_iterator end() const { return this->c.end(); }
};
bool checkValidity(iterable_queue<int>& q) {
 if (q.empty() || q.size() <= 1){
 std::cout << "invalid entry, insufficient elements" << '\n';
 return false;
 }
 while(q.size()){
 auto i = q.begin();
 auto j = ++q.begin();
 for(; i < q.end() && j < ++q.end();){
 std::cout << *i << " " << *j << '\n';
 if (*(i) > *(j)) {
 std::cout << "invalid entry, not properly sorted" << '\n';
 return false;
 }
 i++, j++;
 } 
 std::cout << "valid entry, properly sorted" << '\n';
 return true;
 }
 std::cout << "invalid entry, insufficient elements" << '\n';
 return false;
}
const char* bool_cast(const bool b) {
 return b ? "true" : "false";
} 
int main () {
 iterable_queue<int> numbers;
 int temp;
 std::cout << "Pushing..." << '\n';
 while(temp >= 0){
 std::cout << "Enter numbers: ";
 std::cin >> temp;
 if(temp >= 0){
 numbers.push(temp);
 }
 }
 bool ck = checkValidity(numbers);
 std::cout << bool_cast(ck) << '\n';
 std::cout << "{ ";
 while(numbers.size() > 0){
 std::cout << numbers.front();
 numbers.pop();
 std::cout << " ";
 }
 std::cout << "}" << '\n';
 return 0;
}

• πάντα ῥεῖ: What would happen if q.size() yields zero?
• Darnoc Eloc: Return false. invalid entry, insufficient elements.

Answer 1

Queues are not designed to be used like this. Your iterable_queue clearly shows that you are not using queues the way they are supposed to be used. And the checkValidity function should be simplified with std::is_sorted from the standard library. In q.empty() || q.size() <= 1, the first condition is subsumed by the second. The while(q.size()){ loop also seems to have no significance. ++q.end() is also undefined behavior. The bool_cast functionality is already provided by std::boolalpha.

Overall, I think you are seriously over-complicating everything:

#include <algorithm>
#include <iostream>
#include <iterator>
#include <vector>
int main()
{
 std::vector<int> numbers;
 for (int num; std::cin >> num && num >= 0;) {
 numbers.push_back(num);
 }
 bool valid = numbers.size() > 1 && std::is_sorted(numbers.begin(), numbers.end());
 std::cout << "Valid? " << std::boolalpha << valid << '\n';
 std::cout << "{ ";
 for (int num : numbers) {
 std::cout << num << ' ';
 }
 std::cout << "}\n";
}

(live demo)

• Darnoc Eloc: The point of the exercise is to use the queue container and not a vector.
• L. F.: @DarnocEloc You are deriving from queue and accessing the underlying container directly. That's not how a queue is supposed to work, so in a sense, you are already disregarding "the point of the exercise." I guess the intention of the exercise is to validate the numbers when you read them or when you print them (because traversing a queue is destructive). Anyway, I don't think using queues is an "improvement" for the code, so I'll leave my remarks for other members of the community.