-4
\$\begingroup\$

Here i tried to find the longest possible non repeating sub string in a string in java..plz check my code for errors and give some suggestions for improvement...

    package logicBuilding;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.Scanner;
public class practiceAgain {
    public static void main(String[] args) {
        int length=1,temp=1,storing=0;
        ArrayList a=new ArrayList(100);
        ArrayList tempo=new ArrayList(100);
        String s;
        Scanner sc=new Scanner(System.in);
        System.out.println(" Please enter the string : ");
        s=sc.nextLine();
        for (int i=0;i<s.length();i++) {
            if(!a.contains(s.charAt(i))) {
                length++;
                a.add(s.charAt(i));
            }
            else {
                if(temp<length&&i!=s.length()-1) {
                    temp = length;
                    a.clear();
                    while (storing < i) {
                        a.add(s.charAt(storing));
                        storing++;
                    }
                    tempo=a;
                    a.clear();
                }
                else if(i==s.length()-1) {
                    break;
                }
                    storing=i;
                    length=1;
                    continue;
                }
            }
        System.out.println(" The longest non repeating substring is : ");
        Iterator i1=tempo.iterator();
        while(i1.hasNext()) {
            System.out.print(i1.next());
        }
        }
    }
\$\endgroup\$
4
  • 1
    \$\begingroup\$ You can start by fixing your indentation. \$\endgroup\$ Feb 24 at 11:56
  • 2
    \$\begingroup\$ This Q/A site requires working code, not code for which the execution is questionable. Maybe you could add in some test code so we can see that the basic operation is not in question. There are very basic style issues and all code is in a singular main method. Please check things like Java style guides to get at least a general idea. Or hit "clean up" or "format" in your basic IDE. Using a basic IDE would already give a warning on your package name - the very first line of code. \$\endgroup\$ Feb 24 at 19:44
  • 2
    \$\begingroup\$ Tried lalaland as input, output is not correct. We can close this... \$\endgroup\$ Feb 24 at 21:14
  • 2
    \$\begingroup\$ By the way, I presume you mean "without repeating characters" which is different from just "non-repeating". Be precise, it is important to programmers. \$\endgroup\$ Feb 24 at 21:20

1 Answer 1

1
\$\begingroup\$

The algorithm that you are trying to implement is not described anywhere.

Your code is lacking from a style view:

  • proper indentation;
  • proper amount of newlines;
  • not using spaces where they enhance readability, nor using spaces consistently storing < i is considered good style, i<s.length() isn't;
  • incorrect case for identifiers
    • package name should be reverse identifier like org.jarrar in all lowercase;
    • class name should be CamelCase, not dromedaryCase;
    • braces are not used consistently or according to any style, e.g. using C-style (every brace on it's own line) or the common Java style ( } else { ).
  • declaring multiple variables on the same line is considered bad practice by many Java developers. The code declares variables before they are needed, something that is not required and considered bad style for Java.

The code is lacking methods; methods or functions are the main method for divide and conquer in any code base.

The code is assigning values during declaration even if the value is not required for the code to run properly.

The identifiers do not properly indicate what function they are for, e.g. a is a ArrayList but what is it for? Seemingly tempo seems to have been chosen for the single reason that temp already exists. Same for i1.

The ArrayList nor the Iterator is not parameterized, something that will generate a warning on any Java compiler.

Scanner is a resource that should be used in a Java try-with-resources.

Assigning an ArrayList to another variable (at tempo=a;) is a red flag and is very likely to ensure that the code is invalid or at least inefficient. It also makes the initial assignment of tempo to an ArrayList unnecessary.

The spaces at the start of each output string seem arbitrary. This is not a good way to indicate a prompt.

The arbitrary value 100 is never mentioned to the user, nor is it stored in a constant that explains that it is the maximum line size (which I presume it is if no repetitions are found).

\$\endgroup\$
1
  • \$\begingroup\$ I've generated an answer for you. Please make sure your next submission doesn't violate these common rules. \$\endgroup\$ Feb 24 at 21:15

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