-1

My Java 8 app uses a JTable inside a JScrollPane. Currently the table has more than 10 columns and the data is added using DefaultTableModel's addRow(someObjectArray). All cells in a column currently have the same type of data but columns 4+ can contain cells with "null" (0-3 always contain data != null!).

If the data in the first three columns isn't the same as the data in the previous row, the first three cells in the current row are written in a black, bold font, otherwise they use the normal, plain and dark grey font - it's simply a way to mark the beginning of new data:

enter image description here

Filling the table and scrolling vertically sets the bold font and plain font properly but scrolling right and then back to the first three columns, the bold font is messed up: It's sometimes set for every cell, sometimes for the wrong cells and sometimes it changes mid-text:

enter image description here

Scrolling down and up again resets the cells to what they should look like.

This is how the table is set up:

JTable myTable = new JTable() {
    @Override
    public Component prepareRenderer(TableCellRenderer renderer, int row, int column) {
        Component c = super.prepareRenderer(renderer, row, column);

        if (column<3) {
            if(row==0) { //Always mark column 0-2 in row 0
                c.setFont(c.getFont().deriveFont(Font.BOLD));
                c.setForeground(Color.BLACK);
            } else {
                if(column==0) { //Check if column 0-2 should be marked and mark column 0 (or not)
                    Object prevColumnA = getValueAt(row-1, column); //String
                    Object currColumnA = getValueAt(row, column); //String
                    Object prevColumnB = getValueAt(row-1, column+1); //int
                    Object currColumnB = getValueAt(row, column+1); //int
                    Object prevColumnC = getValueAt(row-1, column+2); //String
                    Object currColumnC = getValueAt(row, column+2); //String

                    if(!prevColumnA.equals(currColumnA) || !prevColumnB.equals(currColumnB) || !prevColumnC.equals(currColumnC)) {
                        markRow = true;
                        c.setFont(c.getFont().deriveFont(Font.BOLD));
                        c.setForeground(Color.BLACK);
                    } else {
                        markRow = false;
                        c.setFont(c.getFont().deriveFont(Font.PLAIN));
                        c.setForeground(Color.DARK_GRAY);
                    }
                } else { //Mark column 1-2 (or not)
                    if(markRow) {
                        c.setFont(c.getFont().deriveFont(Font.BOLD));
                        c.setForeground(Color.BLACK);
                    } else {
                        c.setFont(c.getFont().deriveFont(Font.PLAIN));
                        c.setForeground(Color.DARK_GRAY);
                    }
                }
            }
        } else {
            c.setFont(c.getFont().deriveFont(Font.PLAIN));
            c.setForeground(Color.DARK_GRAY);
        }

        //System.out.println("row="+row+", column="+column+", markRow="+markRow);
        return c;
    }
};

How do I fix this?

Edit: MRE copied from here:

import java.awt.*;
import javax.swing.*;
import javax.swing.table.*;

public class SomeMRE extends JPanel {
    boolean markRow = false;

    public SomeMRE() {
        JTable myTable = new JTable() {
            @Override
            public Component prepareRenderer(TableCellRenderer renderer, int row, int column) {
                Component c = super.prepareRenderer(renderer, row, column);

                if (column<3) {
                    if(row==0) { //Always mark column 0-2 in row 0
                        c.setFont(c.getFont().deriveFont(Font.BOLD));
                        c.setForeground(Color.BLACK);
                    } else {
                        if(column==0) { //Check if column 0-2 should be marked and mark column 0 (or not)
                            Object prevColumnA = getValueAt(row-1, column); //String
                            Object currColumnA = getValueAt(row, column); //String
                            Object prevColumnB = getValueAt(row-1, column+1); //int
                            Object currColumnB = getValueAt(row, column+1); //int
                            Object prevColumnC = getValueAt(row-1, column+2); //String
                            Object currColumnC = getValueAt(row, column+2); //String

                            if(!prevColumnA.equals(currColumnA) || !prevColumnB.equals(currColumnB) || !prevColumnC.equals(currColumnC)) {
                                markRow = true;
                                c.setFont(c.getFont().deriveFont(Font.BOLD));
                                c.setForeground(Color.BLACK);
                            } else {
                                markRow = false;
                                c.setFont(c.getFont().deriveFont(Font.PLAIN));
                                c.setForeground(Color.DARK_GRAY);
                            }
                        } else { //Mark column 1-2 (or not)
                            if(markRow) {
                                c.setFont(c.getFont().deriveFont(Font.BOLD));
                                c.setForeground(Color.BLACK);
                            } else {
                                c.setFont(c.getFont().deriveFont(Font.PLAIN));
                                c.setForeground(Color.DARK_GRAY);
                            }
                        }
                    }
                } else {
                    c.setFont(c.getFont().deriveFont(Font.PLAIN));
                    c.setForeground(Color.DARK_GRAY);
                }

                //System.out.println("row="+row+", column="+column+", markRow="+markRow);
                return c;
            }
        };

        myTable.setModel(new DefaultTableModel(
            new Object[][] {
                {"ColumnA text 1", 123, "ColumnC text 1", 1, null, "bla", "bla", "bla", null},
                {"ColumnA text 2", 234, "ColumnC text 2", 2, null, "bla", "bla", null, null},
                {"ColumnA text 2", 234, "ColumnC text 2", 3, null, "bla", "bla", "bla", null},
                {"ColumnA text 2", 234, "ColumnC text 2", 4, null, "bla", "bla", null, null},
                {"ColumnA text 2", 234, "ColumnC text 2", 5, null, "bla", null, null, null},
                {"ColumnA text 1", 123, "ColumnC text 1", 6, null, "bla", "bla", "bla", null},
                {"ColumnA text 2", 234, "ColumnC text 2", 7, null, "bla", null, null, null},
                {"ColumnA text 2", 234, "ColumnC text 2", 8, null, "bla", "bla", null, null},
                {"ColumnA text 2", 234, "ColumnC text 2", 9, null, "bla", "bla", null, null}
            },
            new String[] {
                "ColumnA", "ColumnB", "ColumnC", "ColumnD", "ColumnE", "ColumnF", "ColumnG", "ColumnH", "ColumnI"
            }
        ));

        setLayout( new BorderLayout() );
        add(new JScrollPane(myTable));
    }

    private static void createAndShowGUI() {
        JFrame frame = new JFrame("SomeMRE");
        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        frame.add(new SomeMRE());
        frame.pack();
        frame.setVisible( true );
    }

    public static void main(String[] args) throws Exception {
        java.awt.EventQueue.invokeLater( () -> createAndShowGUI() );
    }
}
Neph
  • 1,823
  • 2
  • 31
  • 69
  • 3
    (1-) You have been asked in the past to post an [mre] with every question. You didn't so there is not much we can do. I would guess the problem is with your markRow variable. I wouldn't think a variable is necessary. The basic logic should be to set the default rendering then you only ever change the rendering when the bold condition is met. – camickr Oct 29 '19 at 17:54
  • 2
    I second camickr here... a [MCVE] is what is needed to tackle problems like this. camickr's guesses are also very on point. You're not properly configuring the renderer when column>=3 for instance. But that's all we're left with without a [MCVE]: guesses. – TT. Oct 29 '19 at 18:46
  • @TT. How do I "properly configure" the renderer then? I added an `else` case and simply set the font to `Font.Plain` and `Color.DARK_GRAY`, which does change the color of the other columns but the problem's not fixed with that. – Neph Oct 30 '19 at 09:41
  • The else I see in your code is not the else-part for `if( column < 3 )` AFAICT. Also, I doubt your use of `markRow` to mark a specific row is correct. Best to determine settings specifically for each cell, don't count on markRow being correct for another cell. – TT. Oct 30 '19 at 10:27
  • @TT. I added an "else" for `if (column<3)` in my code to test it but I haven't edited the MRE to add it there too yet. I have to check the contents of the first 3 columns, which I only do when it's looking at the very first one. From my understand it iterates over the cells that are currently displayed and then sets the renderer for each. I don't want to do the whole big check `if(column==0)` for every cell, so I need something I can easily check for column 1 and 2 of the same row, which is my `markRow` boolean. – Neph Oct 30 '19 at 10:54
  • How come the question is (still?) marked as "off-topic"? @yur Not passive-aggressive. There have been other things too but I'm not getting into that here. – Neph Oct 30 '19 at 10:58
  • @TT. I added the else to the question for clarity but, like I said, it doesn't seem to make much of a difference. I also added a print just before the `return` and you're right, `markRow` is off once I scroll back to the 0th column: `row=3, column=3, markRow=true` and it displays the 0th and 1st column with plain text but the 2nd in a bold font (this is one of the seemingly random inconsistencies I mentioned). Any idea what's causing this? – Neph Oct 30 '19 at 11:19
  • Best to determine for each cell standalone what its properties should be. Don't count on prepareRenderer being called in a sequential fashion, or any specific order. Setting specific variables and hope prepareRenderer will be called for the "next" cell subsequently will lead to surprises (like what you describe). – TT. Oct 30 '19 at 11:28
  • @TT. Am I right in my assumption that a `List` that stores every `markedRow` or checking column 0 for "boldness" probably won't work then either? Is there a way of making this work without doing the check I currently only do for column 0 for every cell (as it would probably slow everything down)? – Neph Oct 30 '19 at 11:54
  • (1) Looking at the code you posted, it will not slow down in any way you will ever notice. (2) A list with indexes equal to the row indexes (view indexes) could work if the table is static. But then you should take into account that preparing renderers can start with the last row and last cell in the view. I wouldn't bother with that unless calculating cell fonts/color is a significant cost (e.g. a database call to determine a property for a row). – TT. Oct 30 '19 at 13:58
  • (1) I know that the difference in speed probably won't be noticeable (at least not with the naked eye), I just don't like doing the same check over and over if it can be avoided. (2) I see, a list definitely wouldn't work then. No, I only get the data from a database but not the info for formatting, that only depends on the data that's already in the table. (3) I just tested it without `markRow` and the inner `if(column==0)` and it looks like it's working, thanks! – Neph Oct 30 '19 at 14:59
  • @Neph, don't forget to "accept" answers when you get help so people know the problem has been solved. If you think you have a better solution, then post your solution so people can learn. – camickr Oct 30 '19 at 20:06
  • @TT. Want to post an answer? You helped me fix it, so it's only fair if you get the fake internet points for it too. I can add the code I ended up with afterwards if you want. – Neph Oct 31 '19 at 09:47
  • @Neph :-) sure, credibility points as I call them though ;-) – TT. Oct 31 '19 at 11:34
  • @TT. Right, "fake internet points" was Reddit. Want me to add my working code to your answer or should I post it as a new answer? – Neph Oct 31 '19 at 13:05

2 Answers2

2

First of all that in NOT an MRE. Your stated problem was:

but scrolling right and then back to the first three columns, the bold font is messed up:

Well, the code you posted doesn't do horizontal scrolling so there is no way you could have tested the code you posted to make sure it demonstrates your stated problem.

Did you look at my original comment about removing the "markRow" variable and setting the default rendering before testing for the bold conditions?

When I try the above suggestion I was able to simplify the code significantly:

JTable myTable = new JTable()
{
    @Override
    public Component prepareRenderer(TableCellRenderer renderer, int row, int column)
    {
        Component c = super.prepareRenderer(renderer, row, column);
        c.setForeground(Color.DARK_GRAY);

        if (column < 3)
        {
            if(row == 0) { //Always mark columns 0-2 in row 0
                c.setFont(c.getFont().deriveFont(Font.BOLD));
                c.setForeground(Color.BLACK);
            }
            else { // if(column==0) { //Check if column 0-2 should be marked and mark column 0 (or not)
                TableModel model = getModel();
                Object prevColumn0 = model.getValueAt(row-1, 0); //String
                Object currColumn0 = model.getValueAt(row, 0); //String
                Object prevColumn1 = model.getValueAt(row-1, 1); //int
                Object currColumn1 = model.getValueAt(row, 1); //int
                Object prevColumn2 = model.getValueAt(row-1, 2); //String
                Object currColumn2 = model.getValueAt(row, 2); //String

                if (!prevColumn0.equals(currColumn0)
                ||  !prevColumn1.equals(currColumn1)
                ||  !prevColumn2.equals(currColumn2))
                {
                    c.setFont(c.getFont().deriveFont(Font.BOLD));
                    c.setForeground(Color.BLACK);
                }
            }
        }

        return c;
    }
};

From my understand it iterates over the cells that are currently displayed and then sets the renderer for each.

Don't make assumptions about rendering order. Each cell is rendered independently of other cells. This is why you don't want to depend on a variable that is set from a previous cell being rendered.

Edit:

Better getColumnClass(...) implementations would look something like:

@Override
public Class getColumnClass(int column)
{
    switch (column)
    {
        case 2: return Date.class;
        default: return String.class;
    }
}

You would use the above approach when creating a custom TableModel for a POJO. See Row Table Model for a step-by-step approach to create a custom model.

For a more generic implementation that you might use when getting data from a database you could use:

@Override
public Class getColumnClass(int column)
{
    for (int row = 0; row < getRowCount(); row++)
    {
        Object o = getValueAt(row, column);

        if (o != null)
        {
            return o.getClass();
        }
    }

    return Object.class;
}
camickr
  • 321,443
  • 19
  • 166
  • 288
  • I don't know how to set the default rendering (unless it’s with `prepareRenderer`, which I already did). I already managed to fix the code with TT.'s help, just waiting for him to write an answer, so I can accept it. Btw, no need to get the model every time, calling `getValueAt` directly works too. You also don't set the appearance of the cells that don't meet the conditions with your code. `From my understand` means exactly that: "From my understanding". – Neph Oct 31 '19 at 09:58
  • *no need to get the model , calling getValueAt directly works too* - only if the user never reorders the columns. Again, the answers I give promote good long term coding practices to prevent problems when you don't even know you might have problems. Based on the MRE provided it is better to get the data from the model. *also don't set the appearance of the cells that don't meet the conditions with your code* - You are correct. The "bold" attribute overpowered the black/grey color, so I didn't think it was necessary. Added the line back in. – camickr Oct 31 '19 at 14:52
  • (1) You told me to get rid of `markRow`, that's it, no explanation why or how I should change the code. TT helped me fix my code by providing an explanation and answering my questions. (2) I didn't accept your answer because you posted it after I'd already found the solution to my problem with someone else's help (same as this time). It might have been the best answer in your opinion but it doesn't even work the way I need it to (check my comment I posted yesterday). (3) I already told you last time that I don't support re-ordering of columns and never will for this app - this hasn't changed. – Neph Oct 31 '19 at 14:56
  • Also, note I did not reset the Font because the prepareRenderer() method does this. Actually the prepareRenderer should reset the foreground and background as well, but this is a well know bug. – camickr Oct 31 '19 at 14:57
  • *check my comment I posted yesterday* - again, we are not here to provide you with a complete solution, only point you in the right direction. In the small piece of code provided, the suggestion I gave you worked (for that stated problem of that specific question) since all columns contain data. It was not designed to solve all future problems. the main point of the answer was to show you how to do rendering at a "row" instead of a "column" level, which was what the question asked. I can't teach you about every aspect of using a JTable is a couple of lines of code. – camickr Oct 31 '19 at 15:02
  • I'm not asking for a complete solution but if someone posts code, it should work, which yours doesn't because "null" cells throw an exception and there was no obvious fix. I told you multiple times that the data comes from a database, which tend to contain "null" values. If code crashes because of that, it doesn't _promote good long term coding practices_. – Neph Oct 31 '19 at 15:22
  • TT managed to help me without _proper MRE_ and with this help I managed to fix my code, which happened before you posted an answer. That's why he gets the "credit". Let's say I ask about how to bake bread, who do you think has their answer accepted: Someone, who links an actual recipe and answers your questions about it or someone who just posts "flour, water, eggs" and only lateron, once the bread is already in the oven, types out a full recipe? – Neph Oct 31 '19 at 15:29
1

Best is to determine settings specifically for each cell individually in a standalone way, without using a variable (markRow in your code fragment) which is determined at some specific cell coordinate (e.g. the first cell in a row).

The reason is that there is no determined way that prepareRenderer is called. There is no guarantee that this method will first be called for the first cell in the first row, then for the second cell in the first row, and so on. Put differently, don't count on prepareRenderer being called in a sequential fashion, or any specific order.

Also note, that the renderer component should be properly initialized for each cell. In the code fragment from the first version of your question, there was no else part to your starting if(column<3) which means that the component could have been arbitrarily initialized. If there is a decision tree to prepare a renderer, make sure that all paths prepare the renderer as is needed.

TT.
  • 15,774
  • 6
  • 47
  • 88