1

I have the following piece of code:

//code.h
#include<string>
#include<iterator>
#include<iostream>

using std::string;
using std::advance;
using std::iterator;
using std::cin;
using std::cout;
using std::endl;

bool chk_if_uniq (string); 
bool per_scan(iterator,iterator);

//code.cpp
#ifndef CODE_H
#define CODE_H
#include "code.h"
#endif

int main (){
 
 string input_string;

 cout << "enter string: ";
 cin >> input_string;
 
 auto result = chk_if_uniq(input_string);
 
 if(result){
   cout << input_string << " contains unique characters." << endl; 
 }
else{
   cout << input_string << " contains non-unique characters." << endl; 
}

return 0;
}

bool chk_if_uniq (string s){ 
  
  auto bIter = s.begin();
  auto eIter = s.end();
  bool iterPos = (bIter != eIter);
  auto flag = true;

  while(iterPos){
   flag = per_scan(bIter,eIter);
   if(!flag){
     break;
   }
   advance(bIter,1);
  }

 return flag;
}

bool per_scan(iterator it, iterator eIter){

  auto nxIt = it;
  bool iterPos = (nxIt != eIter);
  auto flag = true;

  do{
      ++nxIt;
      if(iterPos){
        if(*nxIt == *it){
          flag = false;
        }
      }
    }while(flag);

  
 return flag;
}

I have the following compilation command:

g++ -ggdb -g3 -o -pedantic-errors -std=c++17 -Wall -Wextra -Wpedantic

The version of gcc that I am using is 8.4.1.

I get the following compiler errors:

In file included from code.cpp:3:
code.h:13:15: error: ‘auto’ parameter not permitted in this context
 bool per_scan(iterator,iterator);
               ^~~~~~~~
code.h:13:24: error: ‘auto’ parameter not permitted in this context
 bool per_scan(iterator,iterator);
                        ^~~~~~~~
code.cpp: In function ‘bool chk_if_uniq(std::__cxx11::string)’:
code.cpp:33:31: error: too many arguments to function ‘bool per_scan()’
    flag = per_scan(bIter,eIter);
                               ^
In file included from code.cpp:3:
code.h:13:6: note: declared here
 bool per_scan(iterator,iterator);
      ^~~~~~~~
code.cpp: At global scope:
code.cpp:43:24: error: ‘auto’ parameter not permitted in this context
 bool per_scan(iterator it, iterator eIter){
                        ^~
code.cpp:43:37: error: ‘auto’ parameter not permitted in this context
 bool per_scan(iterator it, iterator eIter){
                                     ^~~~~
code.cpp: In function ‘bool per_scan()’:
code.cpp:45:15: error: ‘it’ was not declared in this scope
   auto nxIt = it;
               ^~
code.cpp:45:15: note: suggested alternative: ‘int’
   auto nxIt = it;
               ^~
               int
code.cpp:46:27: error: ‘eIter’ was not declared in this scope
   bool iterPos = (nxIt != eIter);
                           ^~~~~
code.cpp:46:27: note: suggested alternative: ‘extern’
   bool iterPos = (nxIt != eIter);
                           ^~~~~
                           extern

As is evident from the error log, all the errors emanate from the usage of iterator as a parameter type for the method per_scan in the header file.

Obviously, my understanding of the iterator concept is flawed.

Can someone point out what is wrong with the usage?

TIA

Vinod
  • 925
  • 8
  • 9
  • 8
    Did you read the manual for `std::iterator`? It's a helper class for writing your own iterators (and it's deprecated), it's not for passing parameters to functions. And it's a template, while you're trying to use it without template arguments. Functions receiving iterators should be templates, with iterator type as template parameter. – HolyBlackCat Aug 02 '23 at 07:03
  • 3
    I suggest you use the same method that all the standard library functions uses: Templates. As in `template bool per_scan(IteratorT,IteratorT);` – Some programmer dude Aug 02 '23 at 07:07
  • 1
    dont place using declarations in headers. You are not `using namespace std;` but if you continue with using every type from `std` that appears in your code then the effect is the same. See [Why is “using namespace std;” considered bad practice?](https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice). Also consider that it puts burden on the reader. For every name I have to go to your list of using declarations before I can know what it refers to. Clean code does not require a reader to jump to different places to understand one place in the code – 463035818_is_not_an_ai Aug 02 '23 at 07:35
  • Iterators in C++ are not polymorphic, and even if they were, polymorphism only works with pointers and references. Unlike other languages like Java or Python, variables in C++ have value semantics, not reference semantics, by default. – Miles Budnek Aug 02 '23 at 08:09

2 Answers2

4

std::iterator is a (deprecated) class template that was used as a helper class when creating iterator classes. What you are looking for is probably a concept called std::input_iterator:

bool per_scan(std::input_iterator auto it, std::input_iterator auto eIter)

However, your per_scan implementation is flawed and causes stack buffer overflow. I suggest using std::find instead of per_scan:

#include <algorithm>

bool chk_if_uniq(const std::string& s) {
    for (auto bIter = s.begin(), eIter = s.end(); bIter != eIter; ++bIter) {
        // replace `per_scan` with `std::find`:
        if(std::find(std::next(bIter), eIter, *bIter) != eIter) return false;
    }
    return true;
}

Alternatively, use std::string::find:

bool chk_if_uniq(const std::string& s) {
    for(std::size_t idx = 0; idx < s.size(); ++idx) {
        if(s.find(s[idx], idx + 1) != std::string::npos) return false;
    }
    return true;
}
Ted Lyngmo
  • 93,841
  • 5
  • 60
  • 108
  • I am not actively using c++20. Unless i have the time to dive into it, I prefer to not use it in answers and leave it for others who know what they are talking about. +1. – 463035818_is_not_an_ai Aug 02 '23 at 08:50
  • @463035818_is_not_an_ai Thanks! I haven't used C++20 much myself. Taking my first babysteps :-) – Ted Lyngmo Aug 02 '23 at 08:53
3

Yes your understanding is flawed. Your usage of the std::iterator template reminds of generics in Java, that are placeholders for the concrete types. They are based on type erasure. Templates on the other hand can be used for type erasure but they don't do it out of the box. A class template must be instantiated before it can be used. The parameter type of a function is a type, not a template. This

bool per_scan(std::iterator,std::iterator);

Makes no sense because std::iterator is not a type.

Moreover, std::iterator is meant to be used as helper when implementing custom iterators. Using an instantiation of std::iterator as function parmeter is rarely useful, as it restricts the parameter type to instantiations of the std::iterator template, but not all iterators fit that. Actually I am not aware of any iterator in the standard library that is an instantiation of std::iterator. One advantage of iterators is that most of the time you do not need to care about the concrete type. For example std::vector::iterator can be implemented as pointer. You need not care.

Consider how eg standard algorithms are declared. The type of the iterators is usually a template argument that makes no assumptions about implementation details (cf eg std::find). The requirements on the implementation are defined in named requirements or via concepts (since C++20). See https://en.cppreference.com/w/cpp/named_req/Iterator.

Change the function into a function template:

template <typename Iter>
bool per_scan(Iter it, Iter eIter){

  auto nxIt = it;
  bool iterPos = (nxIt != eIter);
  auto flag = true;

  do{
      ++nxIt;
      if(iterPos){
        if(*nxIt == *it){
          flag = false;
        }
      }
    }while(flag);

  
 return flag;
}

Live Demo

463035818_is_not_an_ai
  • 109,796
  • 11
  • 89
  • 185