[PATCH] Add dive list view to main window

[Lubomir I. Ivanov]

On 11 April 2013 20:00, Dirk Hohndel <dirk at hohndel.org> wrote:
> "Lubomir I. Ivanov" <neolit123 at gmail.com> writes:
>>
>> hello amit, and thank you for this implementation and pushing the Qt port.
>
> Hear! Hear!
>
> (or was it "Here! Here!"? I can never remember)
>
>> sorry to be too observant, but you could you revise usage of the
>> following (coding style semantics follow):
>> +        if( section == DIVE_NUMBER){
>>
>> and similar. spacing should be:
>> +        if (section == DIVE_NUMBER) {
>
> Yes, please
>
>
>> from:
>> +    DiveItem * itemForIndex(const QModelIndex &) const;
>> to:
>> +    DiveItem *itemForIndex(const QModelIndex &) const;
>>
>> perhaps sticking the * to function or variable name for consistency.
>
> definitely. spaces on both sides if it is a binary operator
> (multiplication), but connected to the element if it is unary
> (pointer).
>
>> something else is usage of C++ pointers:
>> const QString &filename
>> i would use C pointers here. other's opinions?
>
> Ha... my C++ ignorance comes into play... what's the difference?

they are called "references":
http://en.wikipedia.org/wiki/Reference_(C%2B%2B)#Relationship_to_pointers
above page describes differences with pointers. i find them limiting
instead of a better alternative to pointers.

there are big discussions against C pointers in favor of "references", such as:
http://stackoverflow.com/questions/7058339/c-when-to-use-references-vs-pointers

>
>>> +DiveItem::DiveItem(int num, QString dt, float dur, float dep, QString loc, DiveItem *p):
>>> +    m_number(num), m_dateTime(dt), m_duration(dur), m_depth(dep), m_location(loc), m_parent(p)
>> in another thread i pointed out that m_name or m_Name for members is
>> not preferable over mName. perhaps others could comment here as well.
>
> We went a bit back and forth and then decided that members should follow
> the Qt style of naming, i.e., C++ stuff goes the Qt naming (but
> Subsurface whitespace and indentation), C stuff goes Subsurface way of
> naming. And global variable names are based on "where they come from"
> (boy, that vague definition is sure to bite me at some point).
>

i guess i'm OK with the style mSomeVar, then again in C, we don't
really separate globals with function locals.
also henrik, makes a good point that prefixing isn't exactly needed
and i agree with that.

>> on this:
>> +        DiveItem * item = static_cast<DiveItem*>(index.internalPointer());
>> not sure on the final word on C++ casts as well, but at least we can
>> try keeping the * with the following spacing:
>> +        DiveItem *item = static_cast<DiveItem *>(index.internalPointer());
>
> I hate seeing C++ casts. Why would I use them over standard C casts?
>

similar to "references", they are "supposed to be better".

lubomir
--