HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Find largest substring in alphabetical order in a given string

Submitted by: @import:stackexchange-codereview··
0
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 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.