-1

I have the following code below. I can make the first if statement work where they are coloured in red.

I cant make the second if statement work whereby it finds a row with the name "RATS-3" then that row would be coloured in black. Can someone advse?

The third else if statement is executed and colours all the remaining cells green

{
        JTable table = new JTable(array, columnHeaders)
      {
    @Override
            public Component prepareRenderer(TableCellRenderer renderer, int rowIndex,int columnIndex) 
            {
                JComponent component = (JComponent) super.prepareRenderer(renderer, rowIndex, columnIndex);  
    
                if(
                        getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_FFES-2")||
                        getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_FFES-3")  || 
                        getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_FFES-4")  ||
                        getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_FOYE-2")  ||
                        getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_DINO-2") ||
                        getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_DINO-3") ||
                        getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_DINO-4") ||
                        getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_DINO-6")  ||
                        getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_FOYE-1") ||
                        getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_CRUA-1") || 
                        getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_CRUA-2")|| 
                        getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_CRUA-3")||
                        getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_CRUA-4") 
                        
                        && columnIndex == 0) {
                    component.setBackground(Color.RED);
                    
                   if(getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_RATS-3") && columnIndex == 0){
                    component.setBackground(Color.black);  
                    
                    
                }}
                
                 else if(!getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_RATS-4") && columnIndex == 0){
                    component.setBackground(Color.GREEN);
                }
                
                
    
                return component;
            }

Following camickr comments i have amended my if statement but this now colours all the cells red.

  JTable table = new JTable(array, columnHeaders)
      {
       
     @Override
        public Component prepareRenderer(TableCellRenderer renderer, int rowIndex,int columnIndex) 
        {
            JComponent component = (JComponent) super.prepareRenderer(renderer, rowIndex, columnIndex);  

            if(
                    getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_FFES-2")||
                    getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_FFES-3")  || 
                    getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_FFES-4")  ||
                    getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_FOYE-2")  ||
                    getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_DINO-2") ||
                    getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_DINO-3") ||
                    getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_DINO-4") ||
                    getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_DINO-6")  ||
                    getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_FOYE-1") ||
                    getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_CRUA-1") || 
                    getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_CRUA-2")|| 
                    getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_CRUA-3")||
                    getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_CRUA-4") 
                    
                    && columnIndex == 0) {
                component.setBackground(Color.RED);
                
               if(getValueAt(rowIndex, 1).toString().equalsIgnoreCase("T_RATS-3") && columnIndex == 0){
                component.setBackground(Color.black);  
                
                
               }}
            
            

            return component;
        }
Ingram
  • 654
  • 2
  • 7
  • 29
  • (1-) *Following camickr comments i have amended my if statement* - no you haven't. 1) You didn't listen to any of my suggestions to simplify the code. 2) I never suggestion you use separate if statements. I suggested you need an "else" statement. – camickr Jan 01 '21 at 01:15

2 Answers2

2

Note: You should NOT be overriding the prepareRenderer(...) method for something like this. You typically override prepareRenderer(...) when you want to do highlight for all the columns in a given row.

To customize the rendering of a specific column you should be creating a custom renderer and add the renderer to the column.

See: change color JTable cells with getTableCellRendererComponent NOTHING HAPPENDS for a working example.


  1. Why do you have 13 getValueAt(rowIdex, 1).toString() statements?
  2. Why do you use equalsIgnoreCase(...).

You can simplify your logic with something like:

String text = getValueAt(..).toString().toUpperCase();

if (text.equals("T_FFES-s") 
|| (text.equals("..."))
{
    component.setBackground(...);
}

Much less typing and easier to maintain and more efficient.

If you wanted to get really fancy you could add all the string values to a Set as an instance variable in your class). Then you can just use:

String text = getValueAt(..).toString().toUpperCase();

if (mySet.contains(text))
{
    component.setBackground(...);
}

I cant make the second if statement work

Right now you have a nested if statement. It should be an "else statement".

Your logic (simplified) is something like:

if (text.equals("a") || text.equals("b"))
{
    component.setBackground( Color.RED );

    if (text.equals("c"))
    {
        component.setBackground( Color.BLACK );
    }
}

How can the second if condition ever be true, since the logic will never pass the first if condition?

Note you can also simply your if condition by adding logic like:

JComponent component = (JComponent) super.prepareRenderer(renderer, rowIndex, columnIndex);  

if (column != 0)
    return component;

Now your other if conditions don't need to check the column.

The main point of my answer is to simplify the code so it makes it easier to spot logic errors.


Edit:

all the cells remain red

You need to reset the renderer each time to its default value.

The basic code should be:

JComponent component = (JComponent) super.prepareRenderer(renderer, rowIndex, columnIndex);  

component.setBackground( getBackground() ); // this many be needed.

if (column != 0)
    return component;

if (condition1)
    component.setBackground( .... );
else if (condition2)
    component.setBackground(....);

return component;
camickr
  • 321,443
  • 19
  • 166
  • 288
  • I have amended my if statement but this still doesnt work now all the cells remain red. see update to my question – Ingram Dec 31 '20 at 21:32
  • @Ingram, *all the cells remain red* - in addition to all the above suggestions you need to reset the background color to a default value before you do all the if/else tests. See the comment I added at the beginning of the question for the better solution. – camickr Jan 01 '21 at 00:53
1

Your second if statement is inside your first if statement. Therefore it will never be true (since the outer and inner statements have no common Strings):

if(should be red) {
    setToRed();
    if(should be black) { // <- will never be true
        setToBlack();
    }
}

Should be:

if(should be red) {
    setToRed();
}
else if(should be black) { 
    setToBlack();
}

Specifically, you need to end the first if-block after the line:

    component.setBackground(Color.RED);
} // <- end the if block here

And regarding the 13 gets and comparisons, consider something like this:

Set<String> toRed = new HashSet<>(Arrays.asList("T_FFES-2", "T_FFES-3", "T_FFES-4", "T_FOYE-2", "T_DINO-2", "T_DINO-3", "T_DINO-4", "T_DINO-6", "T_FOYE-1", "T_CRUA-1", "T_CRUA-2", "T_CRUA-3", "T_CRUA-4"));
//....
String text = getValueAt(rowIndex, 1).toString.toUpperCase();
if (toRed.contains(text) && columnIndex == 0) {
// ....

Like this:

private static final Set<String> toRed = new HashSet<>(Arrays.asList("T_FFES-2", "T_FFES-3", "T_FFES-4", "T_FOYE-2", "T_DINO-2", "T_DINO-3", "T_DINO-4", "T_DINO-6", "T_FOYE-1", "T_CRUA-1", "T_CRUA-2", "T_CRUA-3", "T_CRUA-4"));

@Override
public Component prepareRenderer(TableCellRenderer renderer, int rowIndex, int columnIndex) {
    JComponent component = (JComponent) super.prepareRenderer(renderer, rowIndex, columnIndex);
    if (columnIndex == 0) {
        String text = getValueAt(rowIndex, 1).toString().toUpperCase();
        if (toRed.contains(text)) {
            component.setBackground(Color.RED);
        } else if (text.equals("T_RATS-3")) {
            component.setBackground(Color.black);
        } else if (!text.equals("T_RATS-4")) {
            component.setBackground(Color.GREEN);
        }
    }
    return component;
}

Edit

Proper else-if code in the example above.

Isolated the if-else code and tested:

public class Tester {

    private static final Set<String> toRed = new HashSet<>(Arrays.asList("T_FFES-2", "T_FFES-3", "T_FFES-4", "T_FOYE-2", "T_DINO-2", "T_DINO-3", "T_DINO-4", "T_DINO-6", "T_FOYE-1", "T_CRUA-1", "T_CRUA-2", "T_CRUA-3", "T_CRUA-4"));

    public static void main(String[] args) {
        List<String> tests = Arrays.asList("T_FFES-2", "t_ffes-3", "T_fFeS-4", "T_RATS-3", "t_RATs-3", "T_raTS-4", "T-Hmmm-0");

        Tester tester = new Tester();
        for (String s : tests) {
            System.out.println(s + ", 0: " + tester.prepareRenderer(s, 0));
        }
        for (String s : tests) {
            System.out.println(s + ", 1: " + tester.prepareRenderer(s, 1));
        }
    }


    public String prepareRenderer(String input, int columnIndex) {
        if (columnIndex == 0) {
            String text = input.toUpperCase();
            if (toRed.contains(text)) {
                return "RED";
            } else if (text.equals("T_RATS-3")) {
                return "BLACK";
            } else if (!text.equals("T_RATS-4")) {
                return "GREEN";
            }
        }
        return "NO COLOR";
    }
}

Test output:

T_FFES-2, 0: RED
t_ffes-3, 0: RED
T_fFeS-4, 0: RED
T_RATS-3, 0: BLACK
t_RATs-3, 0: BLACK
T_raTS-4, 0: NO COLOR
T-Hmmm-0, 0: GREEN
T_FFES-2, 1: NO COLOR
t_ffes-3, 1: NO COLOR
T_fFeS-4, 1: NO COLOR
T_RATS-3, 1: NO COLOR
t_RATs-3, 1: NO COLOR
T_raTS-4, 1: NO COLOR
T-Hmmm-0, 1: NO COLOR

Edit #2

Moved columnIndex == 0 check to separate wrapping if. Makes each if-statement easier to read and skips unnecessary code when columnIndex is not 0.

DarkMatter
  • 1,007
  • 7
  • 13
  • I get a similar issue in that it colours blocks of green then it colours T_RATS-3 Black. Now there is no red so it ignores the first statement. How do i post a screensnapshot? – Ingram Dec 31 '20 at 22:01
  • @Ingram It should be else if on the second if as well (black). Otherwise all the red ones turn green. I will update my answer. – DarkMatter Dec 31 '20 at 23:39
  • Glad you liked all my suggestions, since you repeated them all. – camickr Jan 01 '21 at 00:31
  • @camickr Great minds think alike. Seems I was too focused on writing my answer to notice that you beat me to it. – DarkMatter Jan 01 '21 at 01:05