2
\$\begingroup\$

Here is my approach. Subsequence is a sequence that can be derived from another sequence by deleting some elements without changing the order of the remaining elements.

import java.io.InputStreamReader;
import java.util.Scanner;

public class Query {

public static void main(String[] args) {

    Scanner scanner = new Scanner(new InputStreamReader(System.in));
    String str=scanner.next();
    int n=scanner.nextInt();
    while(n-->0){
        String sub=scanner.next();
        if(isSub(str,sub))
            System.out.println("Yes");
        else
            System.out.println("No");
    }
}

static boolean isSub(String str, String sub){
    int j=0;
    int x=str.length();
    int y=sub.length();
    for(int i=0;i<x && j<y;i++){
        if(str.charAt(i)==sub.charAt(j)) j++;
    }
    return (j==y);
}
}

Please critique my code and also suggest me how to increase efficiency of this code.

\$\endgroup\$
0

2 Answers 2

4
\$\begingroup\$

Formatting

Your code is hard to read when formatted like this:

static boolean isSub(String str, String sub){
    int j=0;
    int x=str.length();
    int y=sub.length();
    for(int i=0;i<x && j<y;i++){
        if(str.charAt(i)==sub.charAt(j)) j++;
    }
    return (j==y);
}

Any IDE can automatically reformat this nicely as:

static boolean isSub(String str, String sub) {
    int j = 0;
    int x = str.length();
    int y = sub.length();
    for (int i = 0; i < x && j < y; i++) {
        if (str.charAt(i) == sub.charAt(j)) {
            j++;
        }
    }
    return (j == y);
}

Naming

The single-letter variables x and y are not easy to follow. Consider this alternative:

static boolean isSub(String source, String sub) {
    int subIndex = 0;
    int sourceLen = source.length();
    int subLen = sub.length();
    for (int i = 0; i < sourceLen && subIndex < subLen; i++) {
        if (source.charAt(i) == sub.charAt(subIndex)) {
            subIndex++;
        }
    }
    return subIndex == subLen;
}

Efficiency

A minor optimization is to first check if the source string is longer than the sub:

static boolean isSub(String source, String sub) {
    int sourceLen = source.length();
    int subLen = sub.length();
    if (sourceLen < subLen) {
        return false;
    }

    int subIndex = 0;
    for (int i = 0; i < sourceLen && subIndex < subLen; i++) {
        if (source.charAt(i) == sub.charAt(subIndex)) {
            subIndex++;
        }
    }
    return subIndex == subLen;
}

Considering that the same source string is reused across multiple isSub tests, there's an opportunity to build some sort of index from the source string.

For example, as a simple optimization, you could build a hash set of characters, so that you can check with an \$O(1)\$ operation if a character in sub cannot appear in the source and return false immediately.

A more advanced index would also consider the search position in the source string. So for example if the source string is "haystackkkkkkkkkk" and the sub is "aaa", then after seeing the 2nd "a", the index can tell you that there will be no more "a" and you can return false immediately.

Scanner

No need to wrap System.in in a InputStreamReader, this works just as well:

Scanner scanner = new Scanner(System.in);
\$\endgroup\$
1
  • \$\begingroup\$ Thank you very much for your time. please provide some references or examples of achieving optimization using hashing. \$\endgroup\$
    – San
    Commented Apr 4, 2016 at 4:25
5
\$\begingroup\$

Formatting

  • Everything inside your class body should be indented by one step further than it currently is.
  • Binary operators (+,-, ...) are usually surrounded by one space to make reading them easier

Outputting results

Your if-else statement can be easily replaced by a ternary statement because it IMO is too trivial to actually be worth the effort of reading 5 lines for it.

In other cases I'd advocate against using the ternary, but here it's just so crudely obvious I can't help but recommend using it.

Immutability

It's significantly cleaner to not allow changing your reference string after you read it in. Mark effectively final variables as final explicitly to ease reading.

After applying this handful of changes your code looks as follows:

public class Query {

    public static void main(String[] args) {
        Scanner scanner = new Scanner(new InputStreamReader(System.in));
        final String str = scanner.next();
        int n = scanner.nextInt();
        while (n-- > 0) {
            String sub = scanner.next();
            System.out.println(isSub(str,sub) ? "Yes" : "No");
        }
    }
}
\$\endgroup\$
2
  • \$\begingroup\$ This solution does not give sub-sequence of String S. For example, S="hello" and q1="elo" . Return output is "No" whereas it should be "Yes" because "elo" is sub-sequence of String S. \$\endgroup\$
    – San
    Commented Apr 3, 2016 at 10:55
  • \$\begingroup\$ @Santosh I misunderstood your question the same way as Vogel, and probably we're not alone. Can you please edit a brief definition of sub-sequence into your question? (In the paragraph just above the code.) Thanks! \$\endgroup\$
    – janos
    Commented Apr 3, 2016 at 11:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.