[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Patch: reducing amount of duplicate code

From: Stefan Fuhrmann <stefanfuhrmann_at_alice-dsl.de>
Date: Mon, 7 Sep 2009 16:31:14 +0200

Dmitry <wipedout_at_yandex.ru> wrote:

> Okay, I've factored out the code for FormatMessage(). Haven't tested it.

Thanks! Committed as r17130-1.

> As for creating .cpp files just to prevent inlining the code of trivial
helper classes I seriously
> don't understand the intent behind that. Inlining is not an inherent
evil by itself and
> introducing an extra file that will consist mainly of the GPL header
doesn't look tempting at all.

All standard C++ language features are legitimate
and I use inlines and templates myself to a great
extend. Maybe, with goto being the exception to
the rule.

Is the use of inlining permissible in your specific
case? Of course! And I wouldn't have accepted the
patch if it wasn't.

Having said that, there are a number of issues that
let me default to not inlining methods (as in
'inline only if there is specific reason to do so').
These are in no particular order

* Many compilers will inline sufficiently simple
  methods only. Marking longer ones as 'inline'
  is therefore misleading.
* Some compilers will inline aggressively. The
  resulting code tends to be *slower* if the inlined
  function has sufficient size. Main reason is the
  increasing number of branch mispredictions.

* Every user of the respective interface will also
  need to parse the implementation. That may offset
  the gains from saving one compilation unit.
* Increases header dependencies resulting in more
  direct and indirect includes. That, too, slows
  down compilation. It may also result in cycles
  later if you had to extend other headers.

* Interfaces tend to grow over time. Once intended
  as a small utility with a (too) narrow scope,
  it may become refactored into a more generic class
  (still with a single, well-defined purpose).
 
Of course, there are a number of situations where
you should inline:

* a given method is trivial (~10 assembly statements,
  no loops)
* the whole class implementation depends on preprocessor
  settings that will vary from one point of usage to
  the other (i.e. prevent linkage issues)

> Moving error handling to one central place is not that obvious. In some
cases CreateProcess()
> errors are silently ignored, in some cases a fixed error message is
shown and in some cases it's a
> custom message - the logic is different each time and factoring it out
is not trivial.

Since that code has been accumulated over many years
and will be invoked in (near-)fatal situations only.
Thus, the quality of the error handling varies greatly.

Silently ignoring CreateProcess() errors should be
wrong in any case. A fixed error message should also
not be optimal in most cases. The user will have no
idea what to do without the system error description.

So, what you may try to implement is a unified 'could
not start tool / process XYZ' message. In the few
cases where a specialized error handling is actually
useful, we can still stick with the current code.

-- Stefan^2.

------------------------------------------------------
http://tortoisesvn.tigris.org/ds/viewMessage.do?dsForumId=757&dsMessageId=2391837

To unsubscribe from this discussion, e-mail: [dev-unsubscribe_at_tortoisesvn.tigris.org].
Received on 2009-09-07 16:31:33 CEST

This is an archived mail posted to the TortoiseSVN Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.