1
\$\begingroup\$

My task is:

In this challenge, write a function CountLOC.count to count the lines of code in a Java source file. A line of code is defined as any line in the source which contains at least one character of executable code. This excludes comments and whitespace. All input will be valid Java code in string format.

CountLOC.count(text) Parameters text: String - A stringified valid Java source file

Return Value int - A count of the number of lines of executable code in the source file

I need help to refactor the logic for the program as:

import java.io.BufferedReader;
import java.io.FileReader;
import java.io.IOException;

public class CountLOC {
    public static void main(String[] args) {
        // write your code here
        System.out.println("Hello world");
    }

    public static int count(String text) {
        boolean currentlyInComment = false;

        int lines = 0;
        try (BufferedReader reader = new BufferedReader(new FileReader(text))) {
            while (reader.readLine() != null) lines++;
            text = text.replace(" ", "");
            String line = reader.readLine();
            if (line.trim().startsWith("/*")) {
                currentlyInComment = true;
            }

            if (!currentlyInComment && !line.trim().startsWith("//")) {
                // Do your algorithmic stuff with line
                System.out.println(line);
            }

            if (line.trim().startsWith("*/") && currentlyInComment) {
                currentlyInComment = false;
            }

            if (line.trim().startsWith("/**") && currentlyInComment) {
                currentlyInComment = false;
            }

            line = reader.readLine();

        } catch (IOException e) {
            e.printStackTrace();
        }
        return lines;
    }
}

Specifically for passing tests below:

import java.io.*;
import java.nio.file.*;
import org.junit.Test;
import static org.junit.Assert.*;

public class ExampleTest {
    @Test
    public void shouldHandleBasicCode() throws IOException {
        String path = "./java_src_files/Example1.java";
        String code = new String(Files.readAllBytes(Paths.get(path)));
        assertEquals(5, CountLOC.count(code));
    }
  
    @Test
    public void shouldHandleABlankLineWIthOneLineComment() throws IOException {
        String path = "./java_src_files/Example2.java";
        String code = new String(Files.readAllBytes(Paths.get(path)));
        assertEquals(5, CountLOC.count(code));
    }
  
    @Test
    public void shouldHandleAnInlineComment() throws IOException {
        String path = "./java_src_files/Example3.java";
        String code = new String(Files.readAllBytes(Paths.get(path)));
        assertEquals(5, CountLOC.count(code));
    }
  
    @Test
    public void shouldHandleMultilineCommentsAndQuotes() throws IOException {
        String path = "./java_src_files/Example4.java";
        String code = new String(Files.readAllBytes(Paths.get(path)));
        assertEquals(5, CountLOC.count(code));
    }
  
    @Test
    public void shouldHandleAComplexExample() throws IOException {
        String path = "./java_src_files/Example5.java";
        String code = new String(Files.readAllBytes(Paths.get(path)));
        assertEquals(6, CountLOC.count(code));
    }
}

I've tried to combine several snippets of the code based on:

  1. https://mkyong.com/java/how-to-get-the-total-number-of-lines-of-a-file-in-java/
  2. https://stackoverflow.com/a/47243579/8370915
  3. https://stackoverflow.com/a/64959634/8370915

But I need to improve it if it possible.

Can someone help me with it, please?

Thank you a lot in advance.

UPD #1:

import java.io.*;

public class CountLOC {
    public static void main(String[] args) {
        // write your code here
        System.out.println("Hello world");
    }

    public static int count(String text) {
        int codeLineCount = 0;
        try {
            BufferedReader reader = new BufferedReader(new FileReader(text));
            String line = reader.readLine();
            while (line != null) {
                if (!isCommentLine(line)) {
                    codeLineCount++;
                }
                line = reader.readLine();
            }
        } catch (IOException e) {
            e.printStackTrace();
        }

        return codeLineCount;
    }
    public static boolean isCommentLine(String line) {
        line = line.trim();
        return line.isEmpty() || line.startsWith("//") || line.startsWith("/*")
                || line.startsWith("*") || line.startsWith("*/");
    }
}

I'm trying to understand how to split the input at the newLine character to get the individual lines and rewrite code to use a string and a loop instead of a reader.

UPD #2:

public class CountLOC {
    
    public static int count(String text) {
        String[] lines = text.split(System.lineSeparator());
        return Arrays.stream(lines).filter(CountLOC::isCode).count();
    }
    public static boolean isCode(String line) {
        line = line.trim();
        return !line.isEmpty() && !line.startsWith("//") && !line.startsWith("/*")
                && !line.startsWith("*") && !line.startsWith("*/");
    }
}
\$\endgroup\$
8
  • \$\begingroup\$ Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$
    – Mast
    Commented Nov 20, 2022 at 11:10
  • \$\begingroup\$ @Mast, got it, thank you for pointing out! \$\endgroup\$
    – invzbl3
    Commented Nov 20, 2022 at 11:11
  • \$\begingroup\$ Ordinarily I'd rollback the edits made after the answer was posted, but while the answer was helpful to you, I'm not convinced it's a review of your code. Considering this site is Code Review, that's important. \$\endgroup\$
    – Mast
    Commented Nov 20, 2022 at 11:11
  • \$\begingroup\$ @Mast, sure, no problem. \$\endgroup\$
    – invzbl3
    Commented Nov 20, 2022 at 11:12
  • \$\begingroup\$ The BufferedReader readLine method reads up to and includes the new line characters. \$\endgroup\$ Commented Nov 20, 2022 at 11:50

3 Answers 3

4
\$\begingroup\$

Prefer try-with-resources

You switched from

        try (BufferedReader reader = new BufferedReader(new FileReader(text))) {

to

        try {
            BufferedReader reader = new BufferedReader(new FileReader(text));

This seems like a mistake. The try-with-resources (first form) is superior. It ensures that the BufferedReader gets closed properly regardless of whether there is an exception or not. The second form doesn't close it at all, so you're guaranteed it won't get closed until the method ends and garbage collection runs. Still consistent, but consistently wrong.

Comments are hard

        return line.isEmpty() || line.startsWith("//") || line.startsWith("/*")
                || line.startsWith("*") || line.startsWith("*/");

This is incorrect. Consider the following input

int product = 5
        * 3;

or

/*
  This is a comment.
*/

The first should be two lines of code (by the definition in the problem). The second should be zero lines of code. But you'd count both as one line of code. Or

/* Comment followed by code */ int product = 5 * 3;

That should be one line of code, but you'd count it as zero lines of code.

If you allow /* */ style comments, then you have to do a lot more parsing and maintain state across lines. Because such comments can appear on lines with executable code or prevent execution of what otherwise would be executable code. You need something like

if (commentOpenFromPreviousLine) {
    int location = line.indexOf("*/");
    if (location >= 0) {
        line = line.substring(location + "*/".length());
        commentOpenFromPreviousLine = false;
    }
}

Note that this is an incomplete solution. For example, it never sets commentOpenFromPreviousLine to true.

Another test case to handle:

String s = """
           /* This is a string, not a comment */
           """;

That should count as three lines of code.

Also

int product = 5 * 3; /* 
    This is a comment
*/

One line of code.

int product /* Comment */ = 5 * 3;

One line of code.

int product /* Comment */ = 5 * 3; /*
       Comment
*/ int sum = 5 + 3;

Two lines of code.

Note: please don't use comments like this in real programs. They're horrible. But they're syntactically allowed.

\$\endgroup\$
3
\$\begingroup\$

It is best to split your code into smaller pieces of code, that do only one thing and to it "right".

@gilbert made your method a bit nicer by having parameter of File type and is improvement, but it can be done even better. One method shouldn't contain bot IO handling (reading from the file) and your logic what you actually want to do.

  • Moving even further, you may want to have a method, that accepts the source code text directly. That way you can parse the code provided by other means. If someone wants to use your code, they currently have to provide files.
  • All the used methods reading contents from files don't specify encoding and by default if the encoding is not specified, platform encoding is used. I believe that java files are utf-8 encoded so it's better to specify that explicitly when loading from them. It's unlikely this would every cause trouble, but I wouldn't take the chance.
  • Exception handling is done poorly. Exceptions are hard to learn, but I would say it's in general better to add throw to the signature or rethrow as RuntimeException. Anything but catching exception and "logging it" and removing something "empty" and not very meaningless In this case just 0. I can't know if it's because of an error, or because the file actually contains 0 lines of meaningful code. If someone wants to use your method and know, if everything went wrong, they don't have a chance.
  • @gilbert's solution doesn't take into consideration multiline comments as your solutions tried to do using currentlyInComment flag. You want to take that into consideration. It is even more complicated. You can have lines, that contain both code and comments. You may want to handle that too, imagine those lines:
System.out.println("I am code" /* comment */); 
System.out.println("I am code");  //I comment
System.out.println("I am code");  /* Start of multiline comment
*/ System.out.println("I am code"); /* another piece of comment */

Your code would consider those as comments and not count them.
  • One way to go is to create regular expressions for all those possibilities and then match each line of code against regexes to determine, if it's code, comment or both. Some examples:
EMPTY = Pattern.compile("\\s*")
BLOCK_COMMENT_ONLY = Pattern.compile("\\s*/[*][^*][*]/\\s*"

You can then use it as EMPTY.matches(line) and handle accordingly by excluding the line or not and determining if you keep being inside the comment or not.

  • Going further, you can create a tokenizer, that handles possible comment starts and endings. Or use regular expressions to detect those.
\$\endgroup\$
2
\$\begingroup\$

I reworked your code to produce the following output.

The program CountLOC has 40 lines of code and 47 total lines

Your main method was incomplete. You combined code from several sources without understanding what each piece of code did.

All you needed was a process to count lines of code and to determine whether or not a line was a comment line.

I created two methods in addition to the main method. The countLOC method counts the code lines and the total lines. The isCommentLine isolates the logic of determining whether or not a line is a comment line. The last startsWith test will never be true. I included it for completeness.

Here's the complete runnable code. You would change the File String to point to a file on your system.

import java.io.BufferedReader;
import java.io.File;
import java.io.FileNotFoundException;
import java.io.FileReader;
import java.io.IOException;

public class CountLOC {

    public static void main(String[] args) {
        CountLOC cl = new CountLOC();
        int[] result = cl
                .countLOC(new File("D:\\Eclipse\\Eclipse-2020-workspace\\"
                        + "com.ggl.testing4\\src\\com\\ggl\\testing\\CountLOC.java"));
        System.out.println("The program CountLOC has " + result[0]
                + " lines of code and " + result[1] + " total lines");
    }

    public int[] countLOC(File file) {
        int lineCount = 0;
        int codeLineCount = 0;
        try {
            BufferedReader reader = new BufferedReader(new FileReader(file));
            String line = reader.readLine();
            while (line != null) {
                lineCount++;
                if (!isCommentLine(line)) {
                    codeLineCount++;
                }
                line = reader.readLine();
            }
        } catch (FileNotFoundException e) {
            e.printStackTrace();
        } catch (IOException e) {
            e.printStackTrace();
        }

        return new int[] { codeLineCount, lineCount };
    }

    public boolean isCommentLine(String line) {
        line = line.trim();
        return line.isEmpty() || line.startsWith("//") || line.startsWith("/*")
                || line.startsWith("*") || line.startsWith("*/");
    }

}
\$\endgroup\$
3
  • \$\begingroup\$ Thank you a lot for your help with refactored code. Could you tell me, please, do you know how this code to combine with attached tests above? \$\endgroup\$
    – invzbl3
    Commented Nov 20, 2022 at 8:45
  • 2
    \$\begingroup\$ @invzbl3: Yes, I do. However, I want you to understand what you're doing and not just blindly copy code from anywhere, including Stack Overflow. Take some time, read the javadocs and tutorials, and learn how Java code works. You have to understand Java to successfully copy and modify code to suit your needs. \$\endgroup\$ Commented Nov 20, 2022 at 11:56
  • \$\begingroup\$ Got it, thank you for your recommendation how to move on. I appreciate it. \$\endgroup\$
    – invzbl3
    Commented Nov 20, 2022 at 12:23

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.