TableListBox - double click in first column does not work


#1

Not sure where to report bugs, so I will just post it here. TableListBox::RowComp::mouseDoubleClick. There is a condition (columnId != 0) that prevents call to TableListBoxModel with column number 0.

Cheers,
J.


#2

It’s not a bug as 0 isn’t a valid ID. This line warns you about adding columns with an ID of 0.


#3

It’s a moderately annoying feature :wink:


#4

I know I will again win “pain in the …” award for the comment… but here we go:
I do not build JUCE in debug because it is just to slow to use it, all those asserts and checks take forever. In release assert is not there - nothing prevented me from creating column with index zero (no exception, crash, nothing). Control works and then again surprise, surpise… I thought only VB guys still count from one, rest of the world use proper C++ blessed counting from zero.

Cheers,
J.


#5

It’s also worth checking the documentation of the addColumn() method here.


#6

You can turn on asserts in your release build, or enable optimisation in your debug build, but for development you REALLY need them turned on.

And no, this isn’t counting from zero - these are not indexes. They’re IDs, and when you have IDs, zero is generally used as the “null” ID.


#7

Say you have a car with manual transmission, you know that you always start from first gear, right. You chnge that car to a different one, it still starts from the first gear. Let’s move now to code world. You take MFC list control, columns start from zero, let’s take wxWidgets grid control, still columns start from zero, now you take JUCE control and ups… they decided that car should start from second gear for some reason :wink:

Cheers,
J.


#8

But as jules pointed out, an ID is not the same as an index, so it is dangerous to rely your code the Id being a sequence starting from 0. IDs can occur in any order, as there might be the option to reorder columns. You can get the mapping to an index using int TableHeaderComponent::getIndexOfColumnId (…). The returned values will start at 0 and count upwards, just as you expect it.
For IDs, there must be an “invalid” value, which happens to be 0. I personally would have preferred -1, but that is the choice of the API designer, and this one leaves the option to store the ID into an unsigned variable (agreed, a weak argument).


#9

I would agree with that if we all lived in perfect wold where you have all the time you need to read documentation, write prototypes, study assertions and so on. Not in the real world when customer is calling every few days asking for progress.

Cheers,
J.


#10

We all hate spending time reading docs, but assertions save you a huge amount of time by catching mistakes that you’d otherwise miss.

All the time you spent debugging this and posting here would have been avoided if you’d hit the assertion that tells you not to use zero IDs!


#11

I think you are too romantic about reading docs :). I could read it if that was say OpenGL control, maybe google map or similar - something really adavnced, but that’s just a grid. It should be simple as brick. It was much easier to just allow column zero and go on.

Cheers,
J.


#12

I agree!

I’d never expect anyone to read docs in order to use such a simple class.

But to call addColumn(), you MUST have looked at it, just to find out what its arguments are:

/** Adds a column to the table.

    This will add a column, and asynchronously call the tableColumnsChanged() method of any
    registered listeners.

    @param columnName       the name of the new column. It's ok to have two or more columns with the same name
    @param columnId         an ID for this column. The ID can be any number apart from 0, but every column must have
                            a unique ID. This is used to identify the column later on, after the user may have
                            changed the order that they appear in
    @param width            the initial width of the column, in pixels
    @param maximumWidth     a maximum width that the column can take when the user is resizing it. This only applies
                            if the 'resizable' flag is specified for this column
    @param minimumWidth     a minimum width that the column can take when the user is resizing it. This only applies
                            if the 'resizable' flag is specified for this column
    @param propertyFlags    a combination of some of the values from the ColumnPropertyFlags enum, to define the
                            properties of this column
    @param insertIndex      the index at which the column should be added. A value of 0 puts it at the start (left-hand side)
                            and -1 puts it at the end (right-hand size) of the table. Note that the index the index within
                            all columns, not just the index amongst those that are currently visible
*/
void addColumn (const String& columnName,
                int columnId,
                int width,
                int minimumWidth = 30,
                int maximumWidth = -1,
                int propertyFlags = defaultFlags,
                int insertIndex = -1);

Could we have made that any clearer?

It even explains WHY it uses an ID rather than an index!

And then, just in case anyone still got it wrong, I put in an assertion which would have instantly caught the mistake!


#13

The assertions are pretty good. Except the ones with snarky comments :wink:


#14

Imagine following use case: User does not read documentation, he goes directly to your demo code, copies your exising table example and adjusts it to his/her needs. That user spent hours doing grids in other frameworks and to make it clear and simple he/she uses enums to name columns, with first value in that enum being zero (it works in other frameworks, why it would not work here). Since JUCE is pretty slow in debug mode, that user also builds it in release mode (no assertions). Now grid is in place, everything is displying as expected, let’s do some interactions. Row selection - works perfeclty from every column. Now double clicking - works but not from column zero, developer thinks… obvious bug - let’s correct that and go on (and maybe drop a line to forum - maybe someone will correct that). I hope you understand me better now :slight_smile:

Cheers,
J.


#15

I’ve explained this multiple times in this thread. So did someone else. It’s also explained in the code itself which I even pasted above for you to see.

For the umpteenth time:

@param columnId         an ID for this column. The ID can be any number apart from 0, but every column must have
                        a unique ID. This is used to identify the column later on, after the user may have
                        changed the order that they appear in

THE ORDER OF THE COLUMNS CAN CHANGE. Therefore, an index is useless because it won’t stay the same. A column needs a UID not an index. That’s what this is. And it’s 100% normal standard practice to use 0 as the null value for a UID or handle type.

I would love to have just used a zero-based index, but that wouldn’t work here. PLEASE tell me you finally understand this!


#16

No I did not :slight_smile: You are saying that all other GUI frameworks have no chance to work because they are not using UIDs … which is obviously not true.

J.


#17

I don’t mean any offense, but this conversation is very confusing. Not everything that is similar is the same as what it is similar to. In fact, that is never the case; hence the reason the word ‘similar’ exists in English.

There is a reason UID is used rather than enum values, as described in the documentation.

Why not try to understand things instead of trying to make them comply to some pre-determined idea of what it should be? Then, after it is understood, if it can be improved, suggest something.


#18

Maybe, just maybe, you should figure out why the debug version is so slow for you (maybe it’s not JUCE, but other functionality?) and rather fix that?

I build exclusively debug versions and still have a giant GUI with tons of widgets, timers, callbacks etc. and not only do it still get 60 Hz, I have tons of CPU left.

You are basically fighting the dev because your wrong expectations are not met, even though the implemented behavior is clearly documented and even caught at runtime (in debug).

For example, if you use std::regex in debug mode, it’s as slow as it gets. It’s unworkable. In release mode it’s OK, not fast, but OK. So i simply stopped using std::regex, and used the String wildcard function.

I think you might have a similar problem that makes the debug version too slow for you.

It’s a lot more productive figuring out alternatives / workarounds, than complaining for hours on the forum about unmet expectations.


#19

I think this will be my last response as I’m repeating myself over and over. @jules implies that UIDs must be used or otherwise there will be no way to identify columns when they change order and since UIDs (whatever they are) are used, there is no way to have column identified by zero, which some people (including me) find odd. I showed usecase (IMHO common for people who write a lot of code and take some assumptions on how stuff should work) when developer may come to a conclusion that this behavior is not right. Hope that helps a little.


#20

I don’t think anyone lacks an understanding of what you are saying. But, do you understand the need for a UID as it relates to columns identification, and the possible re-order of columns (separate from the 1 based UID issue)? I haven’t seen you say that you understand that. Assuming that you do understand that, then the only issue left is that @jules chose to implement the UID starting at 1. Yes, it could have been 0, but it’s not, and doesn’t have to be, because it’s not an index. It’s both documented and protected against via an assert. imo, as a professional developer you only have yourself to blame by not reading the documentation, nor utilizing the available tools (debug mode) to help. I suspect any boss of mine would be unhappy to hear that I spent time trying to fix a bug because I didn’t read the documentation. Or, if your self-employed, I doubt your clients would want to know you wasted time because you didn’t read the documentation. I apologize if this seems overly critical, but you’ve pretty much explained that the problem arose because you couldn’t take the time to read the documentation.