Critique of code quality
in [C++]
|
Prev: How to reuse an existing stream with a different streambuf ?
Next: A quick question about Initializer-List in C++0x
From: Francis Glassborow on 12 Jun 2010 00:26 Peter Olcott wrote: > On 6/11/2010 5:34 PM, Nick Hounsome wrote: >> On 10 June, 21:30, Peter Olcott<NoS...(a)OCR4Screen.com> wrote: >>> On 6/9/2010 2:06 PM, Nick Hounsome wrote: >> >>>>>>>> 3) Eliminate all the global stuff - It's not comp.lang.c here. >>> >>>>>>> I don't know what you mean, everything is a member of a class. >>> >>>>>> all the const uint8_t have global scope >>> >>>>> OK what is the best way to do this? >>> >>>> Move them into the class (as enums) >>> >>> I might get a compile-time warning about assigning a (32-bit?) enum to >>> an 8-bit unsigned char. >> >> Why? You should be assigning an enum to an enum. Your states should be >> enums and your state table should be a table of states. > That would make the table 400% larger, and thus shift my priorities of > readability and efficiency too far in the wrong direction without a > sufficient corresponding benefit. OK but C++0x fixes that problem by allowing the programmer to specify the underlying type of an enum. >> >>>>> MS VS always inlines when you "suggest" it to. >>> >>>> Are you sure? >>>> Have you dissassembled this? >>> Yes. >>>> That's another black mark for MS then. >>> >>> Why? Getting more of exactly what you ask for is a good thing, as long >>> as you know what to ask for. >> >> But in general you don't know what to ask for. >> Do you know exactly what code the compiler will produce for your >> method? >> The compiler does. Therefore it is better placed to decide whether or >> not to inline something. > Inlining is always better for functions that are known to only need to > be called from a single place. Then declare the function static and the compiler has all the information it needs to decide if inlining the code is appropriate. -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: Peter Olcott on 12 Jun 2010 18:59 On 6/12/2010 10:23 AM, Nick Hounsome wrote: > On 12 June, 07:12, Peter Olcott<NoS...(a)OCR4Screen.com> wrote: >> On 6/11/2010 5:34 PM, Nick Hounsome wrote: >>> On 10 June, 21:30, Peter Olcott<NoS...(a)OCR4Screen.com> wrote: >>>> On 6/9/2010 2:06 PM, Nick Hounsome wrote: >> >>>>>>>>> 3) Eliminate all the global stuff - It's not comp.lang.c here. >> >>>>>>>> I don't know what you mean, everything is a member of a class. >> >>>>>>> all the const uint8_t have global scope >> >>>>>> OK what is the best way to do this? >> >>>>> Move them into the class (as enums) >> >>>> I might get a compile-time warning about assigning a (32-bit?) enum to >>>> an 8-bit unsigned char. >> >>> Why? You should be assigning an enum to an enum. Your states should be >>> enums and your state table should be a table of states. >> >> That would make the table 400% larger, and thus shift my priorities of >> readability and efficiency too far in the wrong direction without a >> sufficient corresponding benefit. > > I don't think that the standard requires enums to be stored as int it > just says that they are always promoted to at least int size when used > in an arithmetic context (which they wont be if you compare enum with > enum rather than with uint8_t) so you're table wouldn't be any > bigger. > > The new std allows you to forward declare opaque enums by explicitly > stating the underlying type (which can be smaller than int) so you can > even write stuff like: > > enum State : uint8_t; // State WILL be 1 byte and if any enum value > defined for it wont fit it in uint8_t is a compile time error > struct X { State state; }; // sizeof(X) == 1 OK. I implemented this and the code is cleaner. > > Even if it WERE 300% bigger (400% larger = 5 times the size) it would > make a one off small table that is not on the stack and not on the > heap into a slightly less small table. > I've done a lot of embedded work,some of it on very restricted > machines and I wouldn't think twice about it in any of them. It's just > too small to start thinking of premature optimization. > First write good quality code. Only make it ugly when you have tried > the good stuff and proved that it's too big or too slow. Optimizing for cache locality of reference suggests that the smaller table may be better. This can effect speed. -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: Keith H Duggar on 12 Jun 2010 19:06 On Jun 12, 2:12 am, Peter Olcott <NoS...(a)OCR4Screen.com> wrote: > Inlining is always better for functions that are known > to only need to be called from a single place. In those absolute terms that statement is flat wrong. Whether to inline depends intimately on the relative execution frequency and size of the code in question versus code in the calling chain. It has to do with optimizing instruction cache performance and other factors. See the "Problems" section in the wikipedia http://en.wikipedia.org/wiki/Inline_expansion for a brief summary and vocabulary starting point. KHD -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: Peter Olcott on 13 Jun 2010 05:43 On 6/13/2010 5:06 AM, Keith H Duggar wrote: > On Jun 12, 2:12 am, Peter Olcott<NoS...(a)OCR4Screen.com> wrote: >> Inlining is always better for functions that are known >> to only need to be called from a single place. > > In those absolute terms that statement is flat wrong. Whether to > inline depends intimately on the relative execution frequency and > size of the code in question versus code in the calling chain. It > has to do with optimizing instruction cache performance and other > factors. See the "Problems" section in the wikipedia (1) The ONLY reason not to always inline everything (that can be in-lined) all the time is code bloat. (2) If a function is only called once then there can not possibly be any code bloat at all. (3) Therefore all functions that are only called once should always be in-lined. > > http://en.wikipedia.org/wiki/Inline_expansion > > for a brief summary and vocabulary starting point. > > KHD > -- [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ]
From: Vaclav Haisman on 13 Jun 2010 17:23
Peter Olcott wrote, On 13.6.2010 22:43: >[...] >> In those absolute terms that statement is flat wrong. Whether to >> inline depends intimately on the relative execution frequency and >> size of the code in question versus code in the calling chain. It >> has to do with optimizing instruction cache performance and other >> factors. See the "Problems" section in the wikipedia > > (1) The ONLY reason not to always inline everything (that can be in-lined) > all the time is code bloat. > (2) If a function is only called once then there can not possibly be any code > bloat at all. > (3) Therefore all functions that are only called once should always be in-lined. You have completely ignored the Problems section of the Wikipedia page. Your "reasoning" above is wrong. > >> >> http://en.wikipedia.org/wiki/Inline_expansion >> >> for a brief summary and vocabulary starting point. -- VH [ See http://www.gotw.ca/resources/clcm.htm for info about ] [ comp.lang.c++.moderated. First time posters: Do this! ] |