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