Login | Register
My pages Projects Community openCollabNet

Discussions > dev > Re: Patch - unnecessary actions

Project highlights: :. Download .: :. Support .: :. FAQ .: :. Translations .: :. Donate .: :. Report Bug .:

tortoisesvn
Discussion topic

Hide all messages in topic

All messages in topic

RE: Re: Patch - unnecessary actions

Author klimax
Full name Daniel Klima
Date 2011-08-08 00:25:58 PDT
Message > On 2011-08-04 18:44, Stefan Küng wrote:
> >
> >> Side question how good is VS to use merged strings? From my expirence
> >> using single object (CString) save some conversions and space. This is
> >> minor trought.
> > Can you explain in more detail what you want to do?
> You have many line containing something like:
> String += '\t';
>
> In some languages and compilers there is not direct String + char*
> this make string object creation for '\t' every time you use it. It may
> be marginaly improvement to use String + String. However I use VC only
> for TSVN.
>
> > Stefan
> >

As far as I can tell from source code of MFC*, it allocates new buffer and copies char(s) there.

So no conversion.

*I think MFC is used...

Note:There are several layers of templates and typedefs, but it is down to calling AppendChar:
==
void AppendChar(_In_ XCHAR ch)
    {
        UINT nOldLength = GetLength();
        int nNewLength = nOldLength+1;
        PXSTR pszBuffer = GetBuffer( nNewLength );
        pszBuffer[nOldLength] = ch;
        ReleaseBufferSetLength( nNewLength );
    }
==

Klimax

Re: Patch - unnecessary actions

Author steveking
Full name Stefan Küng
Date 2011-08-07 13:25:27 PDT
Message On 07.08.2011 22:23, Oto BREZINA wrote:
> On 2011-08-04 18:44, Stefan Küng wrote:
>>
>>> Side question how good is VS to use merged strings? From my expirence
>>> using single object (CString) save some conversions and space. This is
>>> minor trought.
>> Can you explain in more detail what you want to do?
> You have many line containing something like:
> String += '\t';
>
> In some languages and compilers there is not direct String + char*
> this make string object creation for '\t' every time you use it. It may
> be marginaly improvement to use String + String. However I use VC only
> for TSVN.

If there was a performance problem here, then of course I'd be ok with
trying to optimize it. But in this case, there's no performance problem.
So I'd like to keep the code as it is - it's easier to read and
understand this way.

Stefan

--
        ___
   oo // \\ "De Chelonian Mobile"
  (_,\/ \_/ \ TortoiseSVN
    \ \_/_\_/> The coolest Interface to (Sub)Version Control
    /_/ \_\ http://tortoisesvn.net

Re: Patch - unnecessary actions

Author otik
Full name Oto BREZINA
Date 2011-08-07 13:23:19 PDT
Message On 2011-08-04 18:44, Stefan Küng wrote:
>
>> Side question how good is VS to use merged strings? From my expirence
>> using single object (CString) save some conversions and space. This is
>> minor trought.
> Can you explain in more detail what you want to do?
You have many line containing something like:
String += '\t';

In some languages and compilers there is not direct String + char*
this make string object creation for '\t' every time you use it. It may
be marginaly improvement to use String + String. However I use VC only
for TSVN.

> Stefan
>

Re: Patch - unnecessary actions

Author steveking
Full name Stefan Küng
Date 2011-08-04 09:45:27 PDT
Message On 04.08.2011 00:26, Oto BREZINA wrote:
> On 2011-08-03 8:05, Dmitry wrote:
>> Hey.
>>
>> Attached is an untested patch that addresses hardcoded constants and also code that uncodnitionally does a change and then conditionaly overrides it instead of doing one of two changes depending on condition.
> According how is defined macro
> ADDTOCLIPBOARDSTRING(x) sClipboard += sClipboard.IsEmpty() ? x : L"\t" + x
>
> sequences like:
> if (selection& SVNSLC_COLREMOTEREVISION)
> {
> if (entry->remoterev<= 0)
> temp.Empty();
> else
> temp.Format(_T("%ld"), entry->remoterev);
> ADDTOCLIPBOARDSTRING(temp);
> }
>
> can become to:
>
> if (selection& SVNSLC_COLREMOTEREVISION)
> {
> if (entry->remoterev> 0)
> {
> temp.Format(_T("%ld"), entry->remoterev);
> ADDTOCLIPBOARDSTRING(temp);
> }
> }
>
> or maybe have two macros one for unconditionally add nonempty string one
> as is would make code even cleaner removing unnecessary tests and adding
> empty strings

Not really. Even if temp is empty (as it's now set if required) the tab
has to be added if the clipboard string is not empty.

Assume sClipboard contains "filename".
after the original sequence, sClipboard is either
filename\t
or
filename\tremoterev

but with your suggested sequence, it would be
filename
or
filename\tremoverev

That's not the same :)

> Side question how good is VS to use merged strings? From my expirence
> using single object (CString) save some conversions and space. This is
> minor trought.

Can you explain in more detail what you want to do?

Stefan

--
        ___
   oo // \\ "De Chelonian Mobile"
  (_,\/ \_/ \ TortoiseSVN
    \ \_/_\_/> The coolest Interface to (Sub)Version Control
    /_/ \_\ http://tortoisesvn.net

Re: Patch - unnecessary actions

Author otik
Full name Oto BREZINA
Date 2011-08-03 15:26:48 PDT
Message On 2011-08-03 8:05, Dmitry wrote:
> Hey.
>
> Attached is an untested patch that addresses hardcoded constants and also code that uncodnitionally does a change and then conditionaly overrides it instead of doing one of two changes depending on condition.
According how is defined macro
ADDTOCLIPBOARDSTRING(x) sClipboard += sClipboard.IsEmpty() ? x : L"\t" + x

sequences like:
          if (selection & SVNSLC_COLREMOTEREVISION)
          {
              if (entry->remoterev <= 0)
                  temp.Empty();
             else
                 temp.Format(_T("%ld"), entry->remoterev);
              ADDTOCLIPBOARDSTRING(temp);
          }

can become to:

          if (selection & SVNSLC_COLREMOTEREVISION)
          {
              if (entry->remoterev > 0)
              {
                 temp.Format(_T("%ld"), entry->remoterev);
                 ADDTOCLIPBOARDSTRING(temp);
              }
          }

or maybe have two macros one for unconditionally add nonempty string one
as is would make code even cleaner removing unnecessary tests and adding
empty strings

Side question how good is VS to use merged strings? From my expirence
using single object (CString) save some conversions and space. This is
minor trought.
> Best wishes.
> Dmitry.
Oto

Patch - unnecessary actions

Author wipedout
Full name Dmitry
Date 2011-08-02 23:05:35 PDT
Message Hey.

Attached is an untested patch that addresses hardcoded constants and also code that uncodnitionally does a change and then conditionaly overrides it instead of doing one of two changes depending on condition.

Best wishes.
Dmitry.
Attachments
Messages per page: