patterncppMinor
Find largest substring in alphabetical order in a given string
Viewed 0 times
ordersubstringlargestfindalphabeticalstringgiven
Problem
#include
#include
using namespace std;
int main()
{
string s, sub;
cin >> s;
int check = 1, count = 0, pos = 0; // keeping count of number of alphabetical ordered substring characters
int i = 1; // loop variable
int prev = 0; //to hold previous value of loop variable
while (i= (int)s[i - 1])
{
++check;
++i;
}
if (check > count)
{
count = check;
pos = i - check;
}
if (i == prev)
++i;
prev = i; //if the inner while loop isn't executed, then i will be incremented here
}
cout << "Longest substring in alphabetical order is :" << s.substr(pos, count) << endl;
}This was a part of my assignment. I could solve this in 10 minutes but I am not sure if this is a very efficient code. Though a (if any) more efficient algorithm exists for this, it may not make much of a different but I am keen on making all my programs as efficient as possible right from beginning.
Solution
Here are a few details :
Variable declaration
It is usually a good idea to move your variable declaration in the smallest possible scope as close as possible to where they are getting used.
This for instance shows that
Also, you can take this chance to change your
Warnings
It is a good thing to activate all warnings. For instance, I get :
warning: comparison between signed and unsigned integer expressions
[-Wsign-compare]
Simplifying things
Instead of messing with
Cast
I do not think these casts are useful.
Length
It could be a good idea to store the length of the string in a variable not to retrieve it at every iteration.
Using namespace
At this stage the code looks like :
Algorithmic trick
A trick could be to say that if you have a current result of length
I am not sure if this is really clear. I'll tr to improve asap.
Variable declaration
It is usually a good idea to move your variable declaration in the smallest possible scope as close as possible to where they are getting used.
This for instance shows that
sub is never used.Also, you can take this chance to change your
while loop into a for loop and declare i in it.Warnings
It is a good thing to activate all warnings. For instance, I get :
warning: comparison between signed and unsigned integer expressions
[-Wsign-compare]
Simplifying things
Instead of messing with
prev, you can detect if i has been increased already by considering the value of check.Cast
I do not think these casts are useful.
Length
It could be a good idea to store the length of the string in a variable not to retrieve it at every iteration.
Using namespace
using namespace is sometimes frowned upon.At this stage the code looks like :
#include
#include
int main()
{
std::string s = "vjvdfkjckvjkjrdsfjsdkjfliureyqnbcxvfjhdeiutogfvjdsvkdfcdcoivjriofdjsrs";
// std::cin >> s;
int count = 0, pos = 0; // keeping count of number of alphabetical ordered substring characters
for (size_t i = 1, len = s.size() ; i = s[i - 1])
{
++check;
++i;
}
if (check > count)
{
count = check;
pos = i - check;
}
if (check == 1)
++i;
}
std::cout << "Longest substring in alphabetical order is :" << s.substr(pos, count) << std::endl;
}Algorithmic trick
A trick could be to say that if you have a current result of length
n, you don't care about any substring of length m < n. Thus, you can check backward from position i + n : as long as it is in descending order, you continue, otherwise, it gives you a starting position to start again.I am not sure if this is really clear. I'll tr to improve asap.
Code Snippets
#include <iostream>
#include <string>
int main()
{
std::string s = "vjvdfkjckvjkjrdsfjsdkjfliureyqnbcxvfjhdeiutogfvjdsvkdfcdcoivjriofdjsrs";
// std::cin >> s;
int count = 0, pos = 0; // keeping count of number of alphabetical ordered substring characters
for (size_t i = 1, len = s.size() ; i < len ; )
{
int check = 1;
while (s[i] >= s[i - 1])
{
++check;
++i;
}
if (check > count)
{
count = check;
pos = i - check;
}
if (check == 1)
++i;
}
std::cout << "Longest substring in alphabetical order is :" << s.substr(pos, count) << std::endl;
}Context
StackExchange Code Review Q#94341, answer score: 4
Revisions (0)
No revisions yet.