current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote -1 down vote favorite

This is my complete code and also I have added logic from Simons answer here selected rows should be 0,1,2,4,5,7,8 but with logic provided by Simon its not working. please suggest over logic in catch block

public static void main(String[] args) 
{
    JFrame frame = new JFrame();
    int[] arr = { 0, 1, 2, 4, 5, 7, 8 };
    frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    JTable t = new JTable(10, 1);
    frame.add(new JScrollPane(t));

    t.getSelectionModel().clearSelection(); 
    frame.pack();
    frame.setVisible(true);    
    try 
    {
        throw new Exception();
    }
    catch (Exception e) 
    {
        int start = -1;
        int end = -1;
        for(int j = 0; j < arr.length - 1; j++) 
        {
            if(arr[j] + 1 == arr[j + 1]) 
            {
                if(start == -1)
                start = j;
                end = j + 1;
            }
            else 
            { 
                t.getSelectionModel().addSelectionInterval(arr[start != -1 ? start : j], arr[end]);
            }
        }
    }
}
share|improve this question

closed as unclear what you're asking by Bobby, rolfl♦, syb0rg, kleinfreund, Simon André Forsberg Feb 3 at 17:11

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question.If this question can be reworded to fit the rules in the help center, please edit the question.
    
is selectedRows an array of int's? I am trying to understand line 5: if(selectedRows[j]+1 == selectedRows[j+1]), also could you provide how the selectedRow is structured. The last line please change it to the following _selectionModel.addSelectionInterval(selectedRows[(start != -1) ? start : j], selectedRows[j]); You have your parenthesis out of place. –  James Feb 3 at 11:59
    
yes selectedRows is array of int. –  eatSleepCode Feb 3 at 12:01
3  
This code will fail when j == SelecterdRows.length - 1 because it will produce ArrayIndexOutOfBoundsException. –  rolfl♦ Feb 3 at 12:14
    
I have updated the question –  eatSleepCode Feb 4 at 9:13
add comment

3 Answers

up vote 2 down vote
  • Your startSelected value is useless.
  • Use the start variable instead and check for -1 or not.
  • It is more clear to use else instead of continue.
  • In java it is convention to put the { on the same line, not on the next
  • Use spaces around + operator to increase readability
int start = -1;
int end = -1;
// avoid ArrayIndexOutOfBounds by looping one index less.
for(int j = 0; j < selectedRows.length - 1; j++) {
    if(selectedRows[j] + 1 == selectedRows[j + 1]) {
        if(start == -1)
           start = j;
        end = j + 1;
        // this if-statement can also be written as `start = Math.max(start, j);`
    }
    else _selectionModel.addSelectionInterval(selectedRows[start != -1 ? start : j], selectedRows[end]);
}
share|improve this answer
    
This code doesn't work as expected. int[] selectedRows = {0,1,2,4,5,7} doesn't select correct rows. –  eatSleepCode Feb 3 at 16:39
    
How it will select last element as loop runs for length -1? –  eatSleepCode Feb 3 at 16:42
    
@eatSleepCode I edited the code a bit. You can use two variables. –  Simon André Forsberg Feb 3 at 17:11
    
still doesn't work for int[] selectedRows = {0,1,2,4,5,7} this case, its not selecting last row. –  eatSleepCode Feb 3 at 17:40
    
with end = j + 1 how can we determine start and end are continuous? –  eatSleepCode Feb 4 at 6:34
add comment
up vote 0 down vote

Your code will throw an ArrayIndexOutOfBoundException. 

if(selectedRows[j]+1 == selectedRows[j+1])

Also could you change the last line to the following

_selectionModel.addSelectionInterval(selectedRows[(start != -1) ? start : j], selectedRows[j]);

The way you put it looks funky. IMHO

share|improve this answer
1  
I agree about the first part, but about the second part I don't agree. In fact, writing it simply as start != -1 ? start : j (without any paranthesis at all) would also be perfectly legal, and something I do myself very often these days. –  Simon André Forsberg Feb 3 at 12:24
    
I am not saying it's not legal I just think it is easier to read because your isolating the condition and the result. –  James Feb 3 at 12:38
    
I have been using that way previously, nowadays I use either paranthesis around everything or nothing at all. log("The value is " + a == b ? "same" : "different" + "."); would be an occation where I always use paranthesis around the whole ternary statement (otherwise you will get different results), but in such a case it would be better to extract the ternary statement and put it in a temporary variable. –  Simon André Forsberg Feb 3 at 13:06
    
When using it as a parameter to a method, or as an index for array, I'd not use paranthesis. When using it in an assignment, I think your approach would be fine a = b == c ? d : e looks weird with all those equality signs. I'm going to stop talking now though before this escalates to a programming holy war ;) –  Simon André Forsberg Feb 3 at 13:08
add comment
up vote -1 down vote accepted

I have added some minor corrections to the Simon André Forsberg answer, now its fine.

        int start = -1;
        for (int i = 0; i < arr.length; i++) 
        {
            if (arr[i] + 1 == arr[arr.length == i+1 ? i : i + 1]) //added this condition for last element.
            {
                if (start == -1)
                start = i;
            }
            else
            {   
                t.getSelectionModel().addSelectionInterval(arr[start != -1 ? start : i], arr[i]);
                start = -1; //setting it again to -1 for next interval.
            }
share|improve this answer
    
Hi, please feel free to mark an answer as accepted if it helped you. I have flagged this post as "not an answer", as it's more of a comment on @Simon's answer. –  Mat's Mug Feb 3 at 17:00
3  
Also, I think you missed an important aspect of @Simon's answer, which is the loop ended at selectedRows.length - 1 ... not selectedRows.length –  rolfl♦ Feb 3 at 17:03
    
@rolfl I intentionally ended loop at selectedRows.length –  eatSleepCode Feb 3 at 17:36
add comment

Not the answer you're looking for? Browse other questions tagged or ask your own question.