Matching Strings to selected rows in JTable

[Tom Hat]

Hey guys,

I have a JTable which shows pdf files. In Addition I can check if they have errors, if it so I can generate a "report" by clicking the pdf. But the report gives me errors from all of the pdfs in the table. How can I match the error messages to the pdf which I select?

Here is one of the checking methods:

public static void testAllPagesHaveSameRotation(PDFDocument pdf) throws PDFDocumentException {

        if (SettingsGui.testAllPagesHaveSameRotation.isSelected()) {

            if (pdf.getPages() == 0) {
                rotationError = "";
            } else {
                rotationError = "";

            }

            int referenceRotation = pdf.getRotation(1).getValue();

            for (int pageNo = 2; pageNo <= pdf.getPages(); pageNo++) {

                if (pdf.getRotation(pageNo).getValue() != referenceRotation) {
                    rotationError = "rotation error" + " | ";
                    errorMessage.append(pdf.getFile().getName() + ": " + "rotation differs on page " + pageNo + "\n");
                }
            }

        } else {
            rotationError = "";
        }
    }

And in the GUI I generate the report so:

errorReport.addActionListener(new ActionListener() {

            @Override
            public void actionPerformed(ActionEvent e) {

                int selectedRow = fileTable.getSelectedRow();

                JDialog errorReport = new JDialog();
                errorReport.setTitle("Error Report " + fileTable.getValueAt(selectedRow, 0));
                JPanel panel = new JPanel();

                // put the labels into a specific panel layout.
                //
                JPanel labelPanel = new JPanel(new GridLayout(0, 1));
                JLabel label = new JLabel("Following errors were found:");
                label.setFont(new Font("Arial", Font.BOLD, 16));
                labelPanel.add(label);
                
                // put the scrollbar into a specific panel layout.

                JScrollPane scroll = new JScrollPane(PDFCheck.errorMessage);
                PDFCheck.errorMessage.setEditable(false);
                PDFCheck.errorMessage.setFont(new Font("Arial", Font.PLAIN, 14));

                // Create a boarder around the items and specify where to show each of the panels previously
                // created.
                //
                panel.add(labelPanel, BorderLayout.NORTH);
                panel.add(scroll, BorderLayout.SOUTH);
                errorReport.add(panel);
                errorReport.pack();

                errorReport.setLocationRelativeTo(null);
                errorReport.setVisible(true);
            }

        });

I hope you understand it a little bit. Thank you!

[Campbell Ritchie]

I think we need more details before we can give you an answer. Please copy and paste the error messages.

This question looks too difficult for the “beginning” forum, so I shall move it.

[Campbell Ritchie]

What happens if you use instructions outside the GUI class to perform the rotation or whatever of the PDFs?
Why do you have an if-else with the same instruction in both halves?

[Campbell Ritchie]

How are you creating the error messages?

[Jacob Anawalt]

So you have a JTable that shows the details of a bunch of PDF files represented as rows in the table and you want to display the Error Report JDialog with only the error messages for the selected row?

It appears that you understand how to catch the event and get the value of the File Name column from your table, but I don't see where you are telling the PDFCheck.errorMessage component to only show messages for errors matching fileTable.getValueAt(selectedRow, 0).

[Tom Hat]

This is my code to check for errors:

   static void testContent(PDFDocument pdf) throws PDFDocumentException {

        if (SettingsGui.testContent.isSelected()) {

            if (pdf.getPages() == 0) {
                contentError = "";
            } else {
                contentError = "";

            }

            for (int pageNo = 1; pageNo < pdf.getPages(); pageNo++) {
                try {
                    PCProperty content = pdf.getContent(pageNo, ContentCollationOptions.NONE);

                    if (content == null) {
                        contentError = "content";

                        // textArea1.append("no content on page " + pageNo + "\n");
                        errorMessage.append(pdf.getFile().getName() + ": " + "no content on page " + pageNo + "\n");

                    }

                } catch (PDFDocumentException exception) {
                    contentError = "content";
                    errorMessage.append(pdf.getFile().getName() + ": " + "failed to read content on page " + pageNo
                            + "\n");

                }
            }

        } else {
            contentError = "";

        }
    }

The problem is that it checks all pdf files which are in a path, which I choose at the beginning of the program. But I just want to check the selected pdf in the table...

Should I try it with this:

fileTable.getValueAt(selectedRow, 6);

--> so I get the path of the pdf and the check can only check this pdf.

[Jacob Anawalt]

I'm sorry, so far the only code you have shared that helps me see what is going on has been your actionPerformed event, and even that, like the others, appears to rely on a lot of instance state. I can only guess what kinds of objects they are and how they are used.

What triggers actionPerformed? A single or a double click of the table row?

Who calls testContent and when?

Is testContent a member of PDFCheck? If not, of what class?

What class is errorMessage, StringBuilder?

I still think the key is to have PDFCheck.errorMessage contain only what you want based on the PDFDocument represented by the selected row.

[Tom Hat]

Jacob Anawalt wrote:I'm sorry, so far the only code you have shared that helps me see what is going on has been your actionPerformed event, and even that, like the others, appears to rely on a lot of instance state. I can only guess what kinds of objects they are and how they are used.

What triggers actionPerformed? A single or a double click of the table row?

Who calls testContent and when?

Is testContent a member of PDFCheck? If not, of what class?

What class is errorMessage, StringBuilder?

I still think the key is to have PDFCheck.errorMessage contain only what you want based on the PDFDocument represented by the selected row.

Sorry for the less information.

What triggers actionPerformed? A single or a double click of the table row?
- It's a double click trigger.

if ((column <= 6) && (me.getClickCount() == 2)) {
                    if (me.getButton() == MouseEvent.BUTTON1) {
..
..

Who calls testContent and when?
- It's called by the class which fills up the ArrayList with data for the JTable. It starts when I choose via JFileChooser the specific path.

Is testContent a member of PDFCheck? If not, of what class?
- Yeah testContent is an own class.

public class PDFCheck extends JPanel {
    static void testContent(PDFDocument pdf) throws PDFDocumentException {
..
..

What class is errorMessage, StringBuilder?

-

JTextArea errorMessage = new JTextArea(25, 45);

I think you're right with your last sentence...but I really don't know how I can implement it...

[Jacob Anawalt]

Ah, that helps clear things up. So JTextArea errorMessage and whatever contentError is must also be static on PDFCheck. JScrollPane(PDFCheck.errorMessage) makes more sense now.

I still don't think the problem is that you parse all the PDF files on load for errors, but that you combine all those errors into one String on the errorMessage JTextArea. I don't think that your static members are helping you see through the design issues. I recommend having testContent return the content error String and an error message String each time it is called on a PDFDocument rather than into static class members that accumulate cruft and have to be reset.

It looks like you stick contentError or something like it into your ArrayList for the JTable. I'm not sure on this part because your screen shot shows more than just blank and "content" in the Error column. Maybe you could drop contentError completely.

You could either stick all of the error message into the ArrayList Error column and pull it back out later for your report, or keep the error messages in another collection, one per row or per PDFDocument with an error, to retrieve and fill a new JTextArea to throw in that new JScrollPane when you double-click a row and look up which row was selected with something like this:

        ....
	String fileName = fileTable.getValueAt(selectedRow, 0);
	JTextArea  text = new JTextArea(pdfChecker.getErrors(fileName)); // or getErrors(int rowIndex);
	JScrollPane scroll = new JScrollPane(text);
        ....

[Tom Hat]

Jacob Anawalt wrote:Ah, that helps clear things up. So JTextArea errorMessage and whatever contentError is must also be static on PDFCheck. JScrollPane(PDFCheck.errorMessage) makes more sense now.

I still don't think the problem is that you parse all the PDF files on load for errors, but that you combine all those errors into one String on the errorMessage JTextArea. I don't think that your static members are helping you see through the design issues. I recommend having testContent return the content error String and an error message String each time it is called on a PDFDocument rather than into static class members that accumulate cruft and have to be reset.

It looks like you stick contentError or something like it into your ArrayList for the JTable. I'm not sure on this part because your screen shot shows more than just blank and "content" in the Error column. Maybe you could drop contentError completely.

You could either stick all of the error message into the ArrayList Error column and pull it back out later for your report, or keep the error messages in another collection, one per row or per PDFDocument with an error, to retrieve and fill a new JTextArea to throw in that new JScrollPane when you double-click a row and look up which row was selected with something like this:

        ....
	String fileName = fileTable.getValueAt(selectedRow, 0);
	JTextArea  text = new JTextArea(pdfChecker.getErrors(fileName)); // or getErrors(int rowIndex);
	JScrollPane scroll = new JScrollPane(text);
        ....

Thank you for your tips! contentError and the other errors that you see are just there to inform the user that there are errors in the pdf files and then in the next step the user should get the report to the selected pdf file with all the pages which are corrupted.

keep the error messages in another collection, one per row or per PDFDocument with an error, to retrieve and fill a new JTextArea to throw in that new JScrollPane when you double-click a row and look up which row was selected

I think that's the simplest way to solve my problem.

My first thought as I created the JTable was to write the errors that correspond to each pdf in a row. But the problem is that pageNo is not correctly printed as String in the JTable, because it's an Int variable...(see picture). The next problem would be the size of the row, because the user just want to see a small part of all the errors and not all.

[Jacob Anawalt]

One way to work around the long string issue would be to setCellRenderer with a custom renderer you extended from DefaultTableCellRenderer that would just print the first N characters, something like "My really long error messages" turns into "My really long..." via the renderer.

If you don't want that kind of text there, then keep putting contentError in and store the error message strings in a different container that you pull the values back out of.

[Tom Hat]

Jacob Anawalt wrote:One way to work around the long string issue would be to setCellRenderer with a custom renderer you extended from DefaultTableCellRenderer that would just print the first N characters, something like "My really long error messages" turns into "My really long..." via the renderer.

If you don't want that kind of text there, then keep putting contentError in and store the error message strings in a different container that you pull the values back out of.

ah okay...I tried out your last suggestion. But the problem is when I store the error message in a different container it will give me again all the errors regardless of the selected pdf file in the table

 if (pdf.getPages() == 0) {
                errorMessage1="";

            } else {
                errorMessage1="";

            }
            StringBuffer stringBuffer = new StringBuffer();

            for (int pageNo = 1; pageNo < pdf.getPages(); pageNo++) {

                try {
                    PCProperty content = pdf.getContent(pageNo, ContentCollationOptions.NONE);

                    if (content == null) {

//                        errorMessage.append(pdf.getFile().getName() + ": " + "no content on page " + pageNo + "\n");

                    }

                } catch (PDFDocumentException exception) {
//                    errorMessage.append(pdf.getFile().getName() + ": " + "failed to read content on page " + pageNo
//                            + "\n");

                    stringBuffer.append("failed to read content on page " + pageNo + "\n");
                    errorMessage1="content error";

                }
                String string = stringBuffer.toString();
                errorMessage = string;
            }

[Jacob Anawalt]

It still looks like you're using global variables, that testContent is not returning values, and I don't know how you are holding and retrieving the correct row from the list of error messages.

I was thinking of something more like this quick & dirty pseudo code example:

String[] testContent(PDFDocument pdf) throws somethingmaybe {
	String contentError;	
	String errorMessage;
	... // all your origional testContent stuff, only not using global contentError and errorMessage
	return new String[] {contentError, errorMessage};
}

// place that is calling testContent and putting
// what were once global variable's content into
// the JTable fileTable's source array
... // I have no idea what this code looks like or how you are doing this
	String[] contentError = testContent(pdf);
	fileTableBackingArrayListRowForPdf.setContentErrorColumn(contentError[0]);
	errorMessageArrayList.add(contentError[0]);
...

// now to pull the right string out for the Error Report dialog
errorReport.addActionListener(new ActionListener() {  
	@Override  
	public void actionPerformed(ActionEvent e) { 
	... // all your first code
	String errorMessage = holderOf.errorMessageArrayList.get(selectedRow);
	// then stuff errorMessage into a new JTextArea that you give to the new JScrolePane
	... // and the rest of your first post code that displays the error message.
});

See what I mean about a second container to store the error messages, and how I pull back out just that PDF's error message for the Error Report dialog?

[Tom Hat]

Jacob Anawalt wrote:It still looks like you're using global variables, that testContent is not returning values, and I don't know how you are holding and retrieving the correct row from the list of error messages.

I was thinking of something more like this quick & dirty pseudo code example:

String[] testContent(PDFDocument pdf) throws somethingmaybe {
	String contentError;	
	String errorMessage;
	... // all your origional testContent stuff, only not using global contentError and errorMessage
	return new String[] {contentError, errorMessage};
}

// place that is calling testContent and putting
// what were once global variable's content into
// the JTable fileTable's source array
... // I have no idea what this code looks like or how you are doing this
	String[] contentError = testContent(pdf);
	fileTableBackingArrayListRowForPdf.setContentErrorColumn(contentError[0]);
	errorMessageArrayList.add(contentError[0]);
...

// now to pull the right string out for the Error Report dialog
errorReport.addActionListener(new ActionListener() {  
	@Override  
	public void actionPerformed(ActionEvent e) { 
	... // all your first code
	String errorMessage = holderOf.errorMessageArrayList.get(selectedRow);
	// then stuff errorMessage into a new JTextArea that you give to the new JScrolePane
	... // and the rest of your first post code that displays the error message.
});

See what I mean about a second container to store the error messages, and how I pull back out just that PDF's error message for the Error Report dialog?

I don't know how you are holding and retrieving the correct row from the list of error messages.

This is my class which fills the JTable with data.

PDFCheck.errorMessage

write all found errors in the cell of the specific pdf:

...
 this.data.add(new Object[] {
                                    file.getName(),
                                    pdfPages,
                                    formatDecimal.format(pdfMediaBoxHeight),
                                    formatDecimal.format(pdfMediaBoxWidth),
                                    formatDecimal.format(pdfTrimBoxHeight),
                                    formatDecimal.format(pdfTrimBoxWidth),
                                    file.getAbsolutePath(),
                                    PDFCheck.rotationError + PDFCheck.widthError + PDFCheck.heightError
                                            + PDFCheck.fontError + PDFCheck.errorMessage });
...

And so I retrieve the errors of the cells:

...
int selectedRow = fileTable.getSelectedRow();

                JTextArea text = new JTextArea(25, 45);
                text.setText((String) fileTable.getValueAt(selectedRow, 7));

                JDialog errorReport = new JDialog();
                JPanel panel = new JPanel();

              
                // put the scrollbar into a specific panel layout.

                final JScrollPane scroll = new JScrollPane(text);
                javax.swing.SwingUtilities.invokeLater(new Runnable() {
                    @Override
                    public void run() {
                        scroll.getVerticalScrollBar().setValue(0);
                    }
                });


                panel.add(scroll, BorderLayout.SOUTH);
                errorReport.add(panel);
                errorReport.pack();
...

[Jacob Anawalt]

It's hard for me as well, looking at the code with blinders on, guessing what is on either side of the snippets.

I'm working off the expectation that the most recently revealed snippet, where you add the row of PDFDocument info to data (PDFCheck.data?) looks something like this, merging what you have revealed to my pseudo code:

...
 String[] contentError = testContent(pdf);
 this.data.add(new Object[] {
                                    file.getName(),
                                    pdfPages,
                                    formatDecimal.format(pdfMediaBoxHeight),
                                    formatDecimal.format(pdfMediaBoxWidth),
                                    formatDecimal.format(pdfTrimBoxHeight),
                                    formatDecimal.format(pdfTrimBoxWidth),
                                    file.getAbsolutePath(),
                                    contentError[0] });
 
 this.errorMessageArrayList.add(PDFCheck.rotationError + PDFCheck.widthError + PDFCheck.heightError
                                            + PDFCheck.fontError + contentError[1]);
...

See how errorMessageArrayList error messages would be in parallel to data? So the index of the one (the same as the index to your JTable row, unless you're sorting or filtering - if so, adjust accordingly) is the same as the index to the other?


It looks like in your latest example you stuck all the error messages into the content error column and then you pull them out from the JTable to create a JTextArea with just that row's error messages. Is that working for you?

[Tom Hat]

Jacob Anawalt wrote:It's hard for me as well, looking at the code with blinders on, guessing what is on either side of the snippets.

I'm working off the expectation that the most recently revealed snippet, where you add the row of PDFDocument info to data (PDFCheck.data?) looks something like this, merging what you have revealed to my pseudo code:

...
 String[] contentError = testContent(pdf);
 this.data.add(new Object[] {
                                    file.getName(),
                                    pdfPages,
                                    formatDecimal.format(pdfMediaBoxHeight),
                                    formatDecimal.format(pdfMediaBoxWidth),
                                    formatDecimal.format(pdfTrimBoxHeight),
                                    formatDecimal.format(pdfTrimBoxWidth),
                                    file.getAbsolutePath(),
                                    contentError[0] });
 
 this.errorMessageArrayList.add(PDFCheck.rotationError + PDFCheck.widthError + PDFCheck.heightError
                                            + PDFCheck.fontError + contentError[1]);
...

See how errorMessageArrayList error messages would be in parallel to data? So the index of the one (the same as the index to your JTable row, unless you're sorting or filtering - if so, adjust accordingly) is the same as the index to the other?


It looks like in your latest example you stuck all the error messages into the content error column and then you pull them out from the JTable to create a JTextArea with just that row's error messages. Is that working for you?

This code snippet from your post is my "FileModel" class which put all the data in the JTable.

Yeah you guess right! At the moment I stuck all the content errors in the "error column" and put them into a JTextArea what works. So I get for each pdf its own error.

I tried again your code, but now step by step:

First this is the class (PDFCheck) to get the error messages in the pdf files:

static String[] testContent(PDFDocument pdf) throws PDFDocumentException {
        String contentError ;
        String errorMessage  ;

        for (int pageNo = 1; pageNo < pdf.getPages(); pageNo++) {

            try {
                PCProperty content = pdf.getContent(pageNo, ContentCollationOptions.NONE);

                if (content == null) {

                    errorMessage = "no content on page " + pageNo + "\n";

                }

            } catch (PDFDocumentException exception) {

                errorMessage = "failed to read content on page " + pageNo + "\n";

//                contentError = "content error";
            }
        }
        return new String[] { contentError, errorMessage };

    }
    String[] contentError = testContent(pdf);  
    fileTableBackingArrayListRowForPdf.setContentErrorColumn(contentError[0]);  
    errorMessageArrayList.add(contentError[0]); 
}

But at the end I don't know where I should put:

 String[] contentError = testContent(pdf);  
    fileTableBackingArrayListRowForPdf.setContentErrorColumn(contentError[0]);  
    errorMessageArrayList.add(contentError[0]);

??

If I put the code in the PDFCheck class I'll get some sytax errors.

Puuuh that's not easy :S

[Jacob Anawalt]

I was envisioning those three lines being in your FileModel class or wherever you are calling static String[] testContent(PDFDocument pdf) and holding this.data and not at the end of PDFCheck.testContent(PDFDocument pdf) (that would be infinitely recursive with no base case.)

Before trying to apply my last suggested changes your program seemed to be meeting it's requirements. Celebrate that and make sure you save a working copy of the code at that point (using some kind of version control system would be great.)

There are design issues, what you may see referred to as "code smell", remaining. If you want to continue to improve your skill at the art of coding well the next step is to identify these issues and refactor. To keep the desired functionality while fixing these design issues. The most glaring one to me is the use of these static class variables which are a Java way of having global variables. It may take stepping back and looking at your overall design and some learning about better designs.

If you aren't convinced, perhaps ask others or search around about global or static class variables to see where they have tripped others up. Add "considered bad" or "considered harmful" or "thread safety" to your search terms.

If you are, then take a look at one or two of your static class variables like PDFCheck.errorMessage and PDFCheck.contentError and think about how all of your code would need to change if they weren't static. Remove the static keyword and look at all the errors. What are the problems? What are ways to accomplish the same user functionality without using static fields (variables)?

It may not be easy yet, but once you learn to think about the problem differently it can be.

[Winston Gutkowski]

Tom Hat wrote:This code snippet from your post is my "FileModel" class which put all the data in the JTable.

Could I make a suggestion? I'm no GUI expert, so I certainly can't help you with the "nuts and bolts" like Jacob has; but it seems to me that you're getting bogged down in the procedural side of things - ie, "how do I code this?" - rather than having (or devising) a plan for what you want to do.

You've clearly done some of that work, but not (I suspect) enough - and if I'm wrong, I'm sure I'll be flamed by the GUI experts here.

Your GUI should ONLY be concerned with display and "reaction" - nothing else - and it really shouldn't care whether you're displaying a PDF, pieces of a Google map, or a set of lottery numbers, or what you want to "do" with them once you've clicked some button or table entry.

That's the theory. I realise that the reality may be somewhat more troublesome; but in most cases it still holds true - a JTable contains "things to be displayed", and it shouldn't matter one whit what they are, or what you intend to do with them.

The problem is that GUis are incredibly verbose and "fiddly", so you tend to get distracted by all the interactions between the various visual (and pointer) components, at the expense of what it is you actually want to do.

My advice: StopCoding (←click) for a bit, and get back to that. Sometimes you can be too close to a problem.

Winston

[Tom Hat]

THANK YOU ALL for your advices. I will heed them.

[Tom Hat]

Winston Gutkowski wrote:

Tom Hat wrote:This code snippet from your post is my "FileModel" class which put all the data in the JTable.

Could I make a suggestion? I'm no GUI expert, so I certainly can't help you with the "nuts and bolts" like Jacob has; but it seems to me that you're getting bogged down in the procedural side of things - ie, "how do I code this?" - rather than having (or devising) a plan for what you want to do.

You've clearly done some of that work, but not (I suspect) enough - and if I'm wrong, I'm sure I'll be flamed by the GUI experts here.

Your GUI should ONLY be concerned with display and "reaction" - nothing else - and it really shouldn't care whether you're displaying a PDF, pieces of a Google map, or a set of lottery numbers, or what you want to "do" with them once you've clicked some button or table entry.

That's the theory. I realise that the reality may be somewhat more troublesome; but in most cases it still holds true - a JTable contains "things to be displayed", and it shouldn't matter one whit what they are, or what you intend to do with them.

The problem is that GUis are incredibly verbose and "fiddly", so you tend to get distracted by all the interactions between the various visual (and pointer) components, at the expense of what it is you actually want to do.

My advice: StopCoding (←click) for a bit, and get back to that. Sometimes you can be too close to a problem.

Winston

I'm not the type of person who will stop coding until the problem is solved.

Wee and now I've solved the problem. It's the quick and dirty way but it works smoothly. I've created another column, set it to invisible and pull out the error messages from this column for each selected pdf file. So I'll get a a nice error reporting in form of a JTextArea with only the errors corresponding to the file.

[Jacob Anawalt]

Nice job keeping after it and solving the visible concerns.

Are you going to continue with solving the design issues? Would you like help understanding how you could do this without creating a table column that you have to set to invisible?

[Tom Hat]

thank you!

oh yes of course I will and it would be a plessure if you can tell it