patterncppMinor
First non-repeated character
Viewed 0 times
firstcharacterrepeatednon
Problem
Challenge description is here need to find the first non repeated character from the string:
Write a program which finds the first non-repeated character in a string.
Input sample:
The first argument is a path to a file. The file contains strings.
For example:
Output sample:
Print to stdout the first non-repeated character, one per line.
For example:
Please let me know can I improve my solution.
Write a program which finds the first non-repeated character in a string.
Input sample:
The first argument is a path to a file. The file contains strings.
For example:
yellow
toothOutput sample:
Print to stdout the first non-repeated character, one per line.
For example:
y
tPlease let me know can I improve my solution.
#include
#include
#include
#include
#include
void findNonRepeatedChar( const std::string& record)
{
auto pred= [&record] ( char a, char b){
return strchr( record.c_str(), a ) stringMap(pred);
for( auto elem : record )
{
auto it = stringMap.find( elem );
if( it != std::end(stringMap) )
{
++(it->second);
}
else
{
stringMap.insert( std::make_pair(elem, 1));
}
}
for( auto elem : stringMap)
{
if(elem.second == 1)
{
std::cout<< elem.first << "\n";
break;
}
}
}
void readInputFile( const char * fileName )
{
std::ifstream inFile( fileName, std::ifstream::in );
std::string record="tooth";
if( !inFile.is_open())
{
std::cout<<"File open failed \n";
return;
}
while( getline( inFile, record ) )
{
findNonRepeatedChar(record);
}
}
int main( int argc, char* argv[] )
{
if( argc < 2 )
{
std::cout<<"Usage:program_name input_file_name \n";
}
readInputFile(argv[1]);
}Solution
I've reviewed your code and here's what I've found.
Reconsider your algorithm
It's not really necessary to iterate through all of the characters in the string, as the current code does. Since the task is to find only the first non-repeated character, the algorithm can terminate as soon as it finds a non-repeated character. A simple way to do that is this:
Reconsider the interface
It is odd that a function named
Don't instantiate an object that is not needed
There is no reason to instantiate the
Use more descriptive function names
The function named
Give the user useful help
The program is probably not really named
Reconsider your algorithm
It's not really necessary to iterate through all of the characters in the string, as the current code does. Since the task is to find only the first non-repeated character, the algorithm can terminate as soon as it finds a non-repeated character. A simple way to do that is this:
void findNonRepeatedChar( const std::string& record )
{
for (std::string::size_type i = 0; i < record.size(); ++i) {
if (record.rfind(record[i]) == i) {
if (record.find(record[i]) == i) {
std::cout << record[i] << '\n';
return;
}
}
}
}Reconsider the interface
It is odd that a function named
findNonRepeatedChar() does not actually return anything. The code could be clearer if it actually returned the non-repeated letter and left the printing to the calling routine.Don't instantiate an object that is not needed
There is no reason to instantiate the
record to "tooth" (or any other value) within readInputFile.Use more descriptive function names
The function named
readInputFile does more than simply read the file and as mentioned before, the findNonRepeatedChar() function doesn't return anything but prints as a side effect. I'd be inclined to omit readInputFile entirely, and put its contents within main and to change the interface to as previously mentioned.Give the user useful help
The program is probably not really named
program_name so the string that's printed if there aren't enough commmand line arguments should probably instead be:std::cout << "Usage: " << argv[0] << " input_file_name\n";Code Snippets
void findNonRepeatedChar( const std::string& record )
{
for (std::string::size_type i = 0; i < record.size(); ++i) {
if (record.rfind(record[i]) == i) {
if (record.find(record[i]) == i) {
std::cout << record[i] << '\n';
return;
}
}
}
}std::cout << "Usage: " << argv[0] << " input_file_name\n";Context
StackExchange Code Review Q#92352, answer score: 6
Revisions (0)
No revisions yet.