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

Follow-up to tool for posting CodeReview questions

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
codereviewpostingquestionsforfollowtool

Problem

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 ReviewPrepar

Solution

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 importSt

Code Snippets

JFileChooser dialog = new JFileChooser();
            dialog.setCurrentDirectory(".");
            dialog.setMultiSelectionEnabled(true);
            dialog.setFileSelectionMode(JFileChooser.FILES_AND_DIRECTORIES);
// 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);
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);
                    }
                });
            }
        }
    };
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) {
private int countLines(File file) throws IOException {
    return Files.readAllLines(file.toPath(), StandardCharsets.UTF_8).size();
}

Context

StackExchange Code Review Q#41225, answer score: 17

Revisions (0)

No revisions yet.