Follow-up to tool for posting CodeReview questions

Question

Description

This is a follow-up question to Tool for creating CodeReview questions.

Things that has changed include:

• Removed replacing four spaces with one tab, all tabs and all spaces in the code itself is now left as-is.
• Added file extensions to the output.
• Switched order of lines and bytes as I feel that the number of lines of code is more interesting than the number of bytes.
• Support for command-line parameters to directly process a directory or a bunch of files, with the support for wildcards. If a directory or wildcard is used, files that don't pass an ASCII-content check gets skipped. If you have specified a file that has a lot of non-ASCII content it is processed anyway.

I am asking for another review because of the things that I have added mostly, see the questions below.

Class Summary (413 lines in 4 files, making a total of 12134 bytes)

• CountingStream.java: OutputStream that keeps track on the number of written bytes to it
• ReviewPrepareFrame.java: JFrame for letting user select files that should be up for review
• ReviewPreparer.java: The most important class, takes care of most of the work. Expects a List of files in the constructor and an OutputStream when called.
• TextAreaOutputStream.java: OutputStream for outputting to a JTextArea.

Code

The code can also be found on GitHub

CountingStream.java: (27 lines, 679 bytes)

/**
 * An output stream that keeps track of how many bytes that has been written to it.
 */
public class CountingStream extends FilterOutputStream {
    private final AtomicInteger bytesWritten;

    public CountingStream(OutputStream out) {
        super(out);
        this.bytesWritten = new AtomicInteger();
    }

    @Override
    public void write(int b) throws IOException {
        bytesWritten.incrementAndGet();
        super.write(b);
    }
    public int getBytesWritten() {
        return bytesWritten.get();
    }
}

ReviewPrepareFrame.java: (112 lines, 3255 bytes)

public class ReviewPrepareFrame extends JFrame {

    private static final long   serialVersionUID    = 2050188992596669693L;
    private JPanel  contentPane;
    private final JTextArea result = new JTextArea();

    /**
     * Launch the application.
     */
    public static void main(String[] args) {
        if (args.length == 0) {
            EventQueue.invokeLater(new Runnable() {
                public void run() {
                    try {
                        new ReviewPrepareFrame().setVisible(true);
                    }
                    catch (Exception e) {
                        e.printStackTrace();
                    }
                }
            });
        }
        else ReviewPreparer.main(args);
    }

    /**
     * Create the frame.
     */
    public ReviewPrepareFrame() {
        setTitle("Prepare code for Code Review");
        setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        setBounds(100, 100, 450, 300);
        contentPane = new JPanel();
        contentPane.setBorder(new EmptyBorder(5, 5, 5, 5));
        contentPane.setLayout(new BorderLayout(0, 0));
        setContentPane(contentPane);

        JPanel panel = new JPanel();
        contentPane.add(panel, BorderLayout.NORTH);

        final DefaultListModel<File> model = new DefaultListModel<>();
        final JList<File> list = new JList<File>();
        panel.add(list);
        list.setModel(model);

        JButton btnAddFiles = new JButton("Add files");
        btnAddFiles.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e) {
                JFileChooser dialog = new JFileChooser();
                dialog.setMultiSelectionEnabled(true);
                if (dialog.showOpenDialog(ReviewPrepareFrame.this) == JFileChooser.APPROVE_OPTION) {
                    for (File file : dialog.getSelectedFiles()) {
                        model.addElement(file);
                    }
                }
            }
        });
        panel.add(btnAddFiles);

        JButton btnRemoveFiles = new JButton("Remove files");
        btnRemoveFiles.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e) {
                for (File file : new ArrayList<>(list.getSelectedValuesList())) {
                    model.removeElement(file);
                }
            }
        });
        panel.add(btnRemoveFiles);

        JButton performButton = new JButton("Create Question stub with code included");
        performButton.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent arg0) {
                result.setText("");
                ReviewPreparer preparer = new ReviewPreparer(filesToList(model));
                TextAreaOutputStream outputStream = new TextAreaOutputStream(result);
                preparer.createFormattedQuestion(outputStream);
            }
        });
        contentPane.add(performButton, BorderLayout.SOUTH);
        contentPane.add(result, BorderLayout.CENTER);
    }

    public List<File> filesToList(DefaultListModel<File> model) {
        List<File> files = new ArrayList<>();
        for (int i = 0; i < model.getSize(); i++) {
            files.add(model.get(i));
        }
        return files;
    }


}

ReviewPreparer.java: (233 lines, 7394 bytes)

public class ReviewPreparer {
    public static double detectAsciiness(File input) throws IOException {
        if (input.length() == 0)
            return 0;
        try (BufferedReader reader = new BufferedReader(new InputStreamReader(new FileInputStream(input)))) {
            int read;
            long asciis = 0;
            char[] cbuf = new char[1024];
            while ((read = reader.read(cbuf)) != -1) {
                for (int i = 0; i < read; i++) {
                    char c = cbuf[i];
                    if (c <= 0x7f)
                        asciis++;
                }
            }
            return asciis / (double) input.length();
        }
    }

    private final List<File> files;

    public ReviewPreparer(List<File> files) {
        this.files = new ArrayList<>();

        for (File file : files) {
            if (file.getName().lastIndexOf('.') == -1)
                continue;

            if (file.length() < 10)
                continue;

            this.files.add(file);
        }
    }

    public int createFormattedQuestion(OutputStream out) {
        CountingStream counter = new CountingStream(out);
        PrintStream ps = new PrintStream(counter);
        outputHeader(ps);
        outputFileNames(ps);
        outputFileContents(ps);
        outputDependencies(ps);
        outputFooter(ps);
        ps.print("Question Length: ");
        ps.println(counter.getBytesWritten());
        return counter.getBytesWritten();
    }

    private void outputFooter(PrintStream ps) {
        ps.println("#Usage / Test");
        ps.println();
        ps.println();
        ps.println("#Questions");
        ps.println();
        ps.println();
        ps.println();
    }

    private void outputDependencies(PrintStream ps) {
        List<String> dependencies = new ArrayList<>();
        for (File file : files) {
            try (BufferedReader in = new BufferedReader(new InputStreamReader(new FileInputStream(file)))) {
                String line;
                while ((line = in.readLine()) != null) {
                    if (!line.startsWith("import ")) continue;
                    if (line.startsWith("import java.")) continue;
                    if (line.startsWith("import javax.")) continue;
                    String importStatement = line.substring("import ".length());
                    importStatement = importStatement.substring(0, importStatement.length() - 1); // cut the semicolon
                    dependencies.add(importStatement);
                }
            }
            catch (IOException e) {
                ps.println("Could not read " + file.getAbsolutePath());
                ps.println();
                // more detailed handling of this exception will be handled by another function
            }

        }
        if (!dependencies.isEmpty()) {
            ps.println("#Dependencies");
            ps.println();
            for (String str : dependencies)
                ps.println("- " + str + ": ");
        }
        ps.println();
    }

    private int countLines(File file) throws IOException {
        return Files.readAllLines(file.toPath(), StandardCharsets.UTF_8).size();
    }

    private void outputFileContents(PrintStream ps) {
        ps.println("#Code");
        ps.println();
        ps.println("This code can also be downloaded from [somewhere](http://github.com repository perhaps?)");
        ps.println();
        for (File file : files) {
            try (BufferedReader in = new BufferedReader(new InputStreamReader(new FileInputStream(file)))) {
                int lines = -1;
                try {
                    lines = countLines(file);
                }
                catch (IOException e) { 
                }
                ps.printf("**%s:** (%d lines, %d bytes)", file.getName(), lines, file.length());

                ps.println();
                ps.println();
                String line;
                int importStatementsFinished = 0;
                while ((line = in.readLine()) != null) {
                    // skip package and import declarations
                    if (line.startsWith("package ")) 
                        continue;
                    if (line.startsWith("import ")) {
                        importStatementsFinished = 1;
                        continue;
                    }
                    if (importStatementsFinished >= 0) importStatementsFinished = -1;
                    if (importStatementsFinished == -1 && line.trim().isEmpty()) // skip empty lines directly after import statements 
                        continue;
                    importStatementsFinished = -2;
                    ps.print("    "); // format as code for StackExchange, this needs to be four spaces.
                    ps.println(line);
                }
            }
            catch (IOException e) {
                ps.print("> Unable to read " + file + ": "); // use a block-quote for exceptions
                e.printStackTrace(ps);
            }
            ps.println();
        }
    }

    private void outputFileNames(PrintStream ps) {
        int totalLength = 0;
        int totalLines = 0;
        for (File file : files) {
            totalLength += file.length();
            try {
                totalLines += countLines(file);
            }
            catch (IOException e) {
                ps.println("Unable to determine line count for " + file.getAbsolutePath());
            }
        }
        ps.printf("###Class Summary (%d lines in %d files, making a total of %d bytes)", totalLines, files.size(), totalLength);
        ps.println();
        ps.println();
        for (File file : files) {
            ps.println("- " + file.getName() + ": ");
        }
        ps.println();
    }

    private void outputHeader(PrintStream ps) {
        ps.println("#Description");
        ps.println();
        ps.println("- Add some [description for what the code does](http://meta.codereview.stackexchange.com/questions/1226/code-should-include-a-description-of-what-the-code-does)");
        ps.println("- Is this a follow-up question? Answer [What has changed, Which question was the previous one, and why you are looking for another review](http://meta.codereview.stackexchange.com/questions/1065/how-to-post-a-follow-up-question)");
        ps.println();
    }

    public static boolean isAsciiFile(File file) {
        try {
            return detectAsciiness(file) >= 0.99;
        }
        catch (IOException e) {
            return true; // if an error occoured, we want it to be added to a list and the error shown in the output
        }
    }

    public static void main(String[] args) {
        List<File> files = new ArrayList<>();
        if (args.length == 0)
            files.addAll(fileList("."));
        for (String arg : args) {
            files.addAll(fileList(arg));
        }
        new ReviewPreparer(files).createFormattedQuestion(System.out);
    }

    public static List<File> fileList(String pattern) {
        List<File> files = new ArrayList<>();

        File file = new File(pattern);
        if (file.exists()) {
            if (file.isDirectory()) {
                for (File f : file.listFiles())
                    if (!f.isDirectory() && isAsciiFile(f))
                        files.add(f);
            }
            else files.add(file);
        }
        else {
            // extract path
            int lastSeparator = pattern.lastIndexOf('\\');
            lastSeparator = Math.max(lastSeparator, pattern.lastIndexOf('/'));
            String path = lastSeparator < 0 ? "." : pattern.substring(0, lastSeparator);
            file = new File(path); 

            // path has been extracted, check if path exists
            if (file.exists()) {
                // create a regex for searching for files, such as *.java, Test*.java
                String regex = lastSeparator < 0 ? pattern : pattern.substring(lastSeparator + 1);
                regex = regex.replaceAll("\\.", "\\.").replaceAll("\\?", ".?").replaceAll("\\*", ".*");
                for (File f : file.listFiles()) {
                    // loop through directory, skip directories and filenames that don't match the pattern
                    if (!f.isDirectory() && f.getName().matches(regex) && isAsciiFile(f)) {
                        files.add(f);
                    }
                }
            }
            else System.out.println("Unable to find path " + file);
        }
        return files;
    }
}

TextAreaOutputStream.java: (41 lines, 806 bytes)

public class TextAreaOutputStream extends OutputStream {

    private final JTextArea textArea;
    private final StringBuilder sb = new StringBuilder();

    public TextAreaOutputStream(final JTextArea textArea) {
        this.textArea = textArea;
    }

    @Override
    public void flush() {
    }

    @Override
    public void close() {
    }

    @Override
    public void write(int b) throws IOException {
        if (b == '\n') {
            final String text = sb.toString() + "\n";
            SwingUtilities.invokeLater(new Runnable() {
                public void run() {
                    textArea.append(text);
                }
            });
            sb.setLength(0);
            return;
        }

        sb.append((char) b);
    }
}

Usage / Test

You can now use the tool directly by downloading the jar-file from GitHub and running it with one of the following options:

• java -jar ReviewPrepare.jar runs the Swing form to let you choose files using a GUI.
• java -jar ReviewPrepare.jar . runs the program in the current working directory and outputting to stdout.
• java -jar ReviewPrepare.jar . > out.txt runs the program in the current working directory and outputting to the file out.txt (I used this to create this question)
• java -jar ReviewPrepare.jar C:/some/path/*.java > out.txt runs the program in the specified directory, matching all *.java files and outputting to the file out.txt

Questions

My main concern currently is with the way I implemented the command line parameters, could it be done easier? (Preferably without using an external library as I would like my code to be independent if possible, although library suggestions for this is also welcome) Is there any common file-pattern-argument that I missed?

I'm also a bit concerned with the extensibility of this, right now it feels not extensible at all. What if someone would want to add custom features for the way Python/C#/C++/etc. files are formatted? Then hard-coding the "scan for imports" in the way I have done it doesn't feel quite optimal.

General reviews are also of course welcome.

Answer 1

General

Now that you have such neat postings, the answers are going to need to be neater too.

GUI Bugs

When I run the GUI, it does not let me select directories from the File Browser. It also starts in the 'Documents' directory, and it would be better to do one of two things:

• start in the current directory
• start in the last directory used (use java.util.pefs.Preferences ?)

You should add:

            JFileChooser dialog = new JFileChooser();
            dialog.setCurrentDirectory(".");
            dialog.setMultiSelectionEnabled(true);
            dialog.setFileSelectionMode(JFileChooser.FILES_AND_DIRECTORIES);

Then you should also support expanding any directory results from the chooser. This will make the behaviour in the GUI match the commandline more closely.

A second problem is in the JTextArea display. It should have scroll-bars so that you can inspect the results before copying/pasting them. While looking at those changes, I discovered that you were doing all your File IO on the event-dispatch thread... this is bad practice....

I had to do the following:

// add a scrollPane....
private final JScrollPane scrollPane = new JScrollPane(result);

......

// Inside the constructor:

    final Runnable viewupdater = new Runnable() {
        public void run() {
            result.setText("");
            ReviewPreparer preparer = new ReviewPreparer(filesToList(model));
            TextAreaOutputStream outputStream = new TextAreaOutputStream(result);
            preparer.createFormattedQuestion(outputStream);
            outputStream.flush();
            result.setCaretPosition(0);
        }
    };

    JButton performButton = new JButton("Create Question stub with code included");
    performButton.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent arg0) {
            Thread worker = new Thread(viewupdater);
            worker.setDaemon(true);
            worker.start();
        }
    });

    scrollPane.setAutoscrolls(true);
    scrollPane.setHorizontalScrollBarPolicy(ScrollPaneConstants.HORIZONTAL_SCROLLBAR_AS_NEEDED);
    scrollPane.setVerticalScrollBarPolicy(ScrollPaneConstants.VERTICAL_SCROLLBAR_AS_NEEDED);
    contentPane.add(scrollPane, BorderLayout.CENTER);
    contentPane.add(performButton, BorderLayout.SOUTH);

As I was doing this change I noticed that you are not doing any best-practice closing of the TextAreaOutputStream instance, and, I looked in to the TextAreaOutputStream code, and, it's not the right solution. It is creating a new thread for every line from every file.... and it is horrible overkill. That whole class should be removed, and replaced with:

    final Runnable viewupdater = new Runnable() {
        public void run() {
            ReviewPreparer preparer = new ReviewPreparer(filesToList(model));
            try (final StringWriter sw = new StringWriter()) {
                preparer.createFormattedQuestion(sw);
                SwingUtilities.invokeLater(new Runnable() {
                    @Override
                    public void run() {
                        result.setText(sw.toString());
                        result.setCaretPosition(0);
                    }
                });
            }
        }
    };

Note how the above is changed to use a Writer instead of an OutputStream..... Using an OutputStream for text data is a broken model.... Readers and Writers for text, and Streams for binary.

That's a good segway in to the non-gui code....

The Core engine

The TextareaOutputStream made me realize that all of your methods are stream based, except for some parts that are buried in the ReviewPreparer.

The PrintStream code should all be replaced with a StringBuilder..... you are limited to the size of a CR post anyway, and you are accumulating the data in to a TextArea... it's not like you will run out of memory.

This is also an interesting segway to the CountingOutputStream. There is no need for that either.... you are not using it to count the file sizes, but the actual post length. This should be measured in characters, not bytes.... so, it's a broken class. Get rid of it.

So, get rid of the PrintStream as well. PrintStream is a synchronized class, and is much, much slower than StringBuilder. Appending the data to StringBuilder also means you can get the character-length from the StringBuilder instead of the byte-length from the CountingOutputStream.

One final observation....... inside the outputFileContents(PrintStream ps) method you do:

        try (BufferedReader in = new BufferedReader(new InputStreamReader(
                new FileInputStream(file)))) {
            int lines = -1;
            try {
                lines = countLines(file);
            } catch (IOException e) {
            }
            ps.printf("**%s:** (%d lines, %d bytes)", file.getName(),
                    lines, file.length());

            ps.println();
            ps.println();
            String line;
            int importStatementsFinished = 0;
            while ((line = in.readLine()) != null) {

This is broken for a few reasons....

Firstly, you should not be using a FileInputStream, but a FileReader.

Secondly, you have the support method countLines(File):

private int countLines(File file) throws IOException {
    return Files.readAllLines(file.toPath(), StandardCharsets.UTF_8).size();
}

This method fully-reads the file.... again .... Why don't you replace all the big code above with:

        try {
            List<String> filelines = Files.readAllLines(file.toPath(), StandardCharsets.UTF_8);
            sb.append(String.format("**%s:** (%d lines, %d bytes)", file.getName(),
                    filelines.size(), file.length()));

            sb.append("\n\n");
            int importStatementsFinished = 0;
            for (String line : filelines) {
                // skip package and import declarations

This saves having to read each file twice.....

Anway, that's enough for now.