Code review

This is a discussion on Code review within the C++ Programming forums, part of the General Programming Boards category; Originally Posted by Elysia Sure, it's what I do. But classes are better than free functions. In what way? And ...

  1. #16
    Banned
    Join Date
    Nov 2007
    Posts
    678
    Quote Originally Posted by Elysia View Post
    Sure, it's what I do. But classes are better than free functions.
    In what way?

    And a side note (I know you will hate to do this), if you wanted a better string class, which is, supposedly, the main reason you are writing your own class, then, you should try QString, you will fall in love with me, for suggesting this to you

    Try!

  2. #17
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,662
    Does it overload operators for integral types?
    Does it overload a split function?
    Does it overload a function to conver to/from unicode and ansi?

    Classes are superior because they can use RAII and are OOP. It's better to do name.GetLength() instead of GetLength(name) IMHO.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  3. #18
    Banned
    Join Date
    Nov 2007
    Posts
    678
    Quote Originally Posted by Elysia View Post
    Does it overload operators for integral types?
    Does it overload a split function?
    Does it overload a function to conver to/from unicode and ansi?

    Classes are superior because they can use RAII and are OOP. It's better to do name.GetLength() instead of GetLength(name) IMHO.
    Sure enough, that, I absolutely agree!

    But after you are done with your CStringEx, I will have three incomplete classes to choose from:
    std::string, CString & CStringEx - pick one!
    I pick - QString

  4. #19
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,662
    Well, that's you. This one's for me.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  5. #20
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,662
    I'm bumping into problems here, especially with the template specialization:

    My definition is as follows:
    Code:
     template<typename T> class StrTraits
    	{
    	protected:
    		typedef int (fnc_tprintf_s)(T* buffer, size_t sizeOfBuffer, const T* format, ...);
    		typedef int (fnc_tvprintf_s)(T* buffer, size_t sizeOfBuffer, const T* format, va_list argptr);
    		typedef int (fnc_tcscmp)(const T* string1, const T* string2);
    		typedef int (fnc_tvcprintf)(const T* format, va_list argptr);
    
    		static const T* UINT64_T_IDENTIFIER;
    		static const T* INT64_T_IDENTIFIER;
    		static const T* LONG_IDENTIFIER;
    
    		uint32_t GetLength(const T* strData) const; // { return strlen(strData); }
    		fnc_tprintf_s* GetFormatFunction(const T*) const;
    		fnc_tvprintf_s* GetVFormatFunction(const T*) const;
    		fnc_tvcprintf* GetVFormatCountFunction(const T*) const;
    		fnc_tcscmp* GetCompareFunction(const T*) const;
    	};
    And the template specialization is later:

    Code:
    template<> uint32_t StrTraits<char>::GetLength(const char* strData) const { return strlen(strData); }
    template<> uint32_t StrTraits<wchar_t>::GetLength(const wchar_t* strData) const { return wcslen(strData); }
    template<> typename StrTraits<char>::fnc_tprintf_s* StrTraits<char>::GetFormatFunction(const char*) const { return &sprintf_s; }
    template<> typename StrTraits<wchar_t>::fnc_tprintf_s* StrTraits<wchar_t>::GetFormatFunction(const wchar_t*) const { return &swprintf_s; }
    template<> typename StrTraits<char>::fnc_tvprintf_s* StrTraits<char>::GetVFormatFunction(const char*) const { return &vsprintf_s; }
    template<> typename StrTraits<wchar_t>::fnc_tvprintf_s* StrTraits<wchar_t>::GetVFormatFunction(const wchar_t*) const { return &vswprintf_s; }
    template<> typename StrTraits<char>::fnc_tvcprintf* StrTraits<char>::GetVFormatCountFunction(const char*) const { return &_vscprintf; }
    template<> typename StrTraits<wchar_t>::fnc_tvcprintf* StrTraits<wchar_t>::GetVFormatCountFunction(const wchar_t*) const { return &_vscwprintf; }
    template<> typename StrTraits<char>::fnc_tcscmp* StrTraits<char>::GetCompareFunction(const char*) const { return &strcmp; }
    template<> typename StrTraits<wchar_t>::fnc_tcscmp* StrTraits<wchar_t>::GetCompareFunction(const wchar_t*) const { return &wcscmp; }
    But it gives me load of linking errors for doing so. Multiple symbols!

    Code:
    error LNK2005: "protected: unsigned int __thiscall Strings::StrTraits<char>::GetLength(char const *)const " already defined in Stuff.obj
    error LNK2005: "protected: unsigned int __thiscall Strings::StrTraits<wchar_t>::GetLength(wchar_t const *)const " already defined in Stuff.obj
    error LNK2005: "protected: int (__cdecl*__thiscall Strings::StrTraits<char>::GetFormatFunction(char const *)const )(char *,unsigned int,char const *,...)" already defined in Stuff.obj
    error LNK2005: "protected: int (__cdecl*__thiscall Strings::StrTraits<wchar_t>::GetFormatFunction(wchar_t const *)const )(wchar_t *,unsigned int,wchar_t const *,...)" already defined in Stuff.obj
    error LNK2005: "protected: int (__cdecl*__thiscall Strings::StrTraits<char>::GetVFormatFunction(char const *)const )(char *,unsigned int,char const *,char *)" already defined in Stuff.obj
    error LNK2005: "protected: int (__cdecl*__thiscall Strings::StrTraits<wchar_t>::GetVFormatFunction(wchar_t const *)const )(wchar_t *,unsigned int,wchar_t const *,char *)" already defined in Stuff.obj
    error LNK2005: "protected: int (__cdecl*__thiscall Strings::StrTraits<char>::GetVFormatCountFunction(char const *)const )(char const *,char *)" already defined in Stuff.obj
    error LNK2005: "protected: int (__cdecl*__thiscall Strings::StrTraits<wchar_t>::GetVFormatCountFunction(wchar_t const *)const )(wchar_t const *,char *)" already defined in Stuff.obj
    error LNK2005: "protected: int (__cdecl*__thiscall Strings::StrTraits<char>::GetCompareFunction(char const *)const )(char const *,char const *)" lready defined in Stuff.obj
    error LNK2005: "protected: int (__cdecl*__thiscall Strings::StrTraits<wchar_t>::GetCompareFunction(wchar_t const *)const )(wchar_t const *,wchar_t const *)" already defined in Stuff.obj
    All I can say is that template specialization is not my strong side >_<
    (Sorry for the long linking errors)

    Further, if I do the implementation inline in the definition and add one specialization later, it works fine.
    If I split the implementation to outside the definition (like how I did with CTmplStringBase), it also gives errors.

    EDIT: Dammit, now I'm even having more problems.
    All of the template functions which are split from the definition fails to work.
    Last edited by Elysia; 05-10-2008 at 05:33 AM.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  6. #21
    Registered User
    Join Date
    May 2008
    Location
    Paris
    Posts
    248
    Why do you derive your template class from your traits template? Usually the trait is used as a template parameter in a template class, since this latter is the whole reason for making traits!
    Further, I think that typedefs are better than declaring static variables? Static variables are more used by value traits, e.g. define a default initializer. Your traits are more type traits.

    I would expect something like:

    Code:
    //general traits declaration
    template<> 
    struct StrTraits;
    
    // trait specialized
    template<UINT64_T_IDENTIFIER> 
    struct StrTraits
    {
    typedef UINT64_T_IDENTIFIER myIdentifier;
    };
    
    template<typename T, typename traits = StrTraits<T> >
    class CTmplStringBase:
    {
    public:
    CTmplStringBase(const CStringT< StrTraitMFC_DLL<typename traits::myIdentifier> >& strSrc);
    };
    Or did I miss the whole point?

    ps: a nice book about this is C++ Templates, Vandevoorde&Josuttis, 2003.
    if anyone knows a similar or maybe even better book....
    Last edited by MarkZWEERS; 05-10-2008 at 06:13 AM. Reason: reference

  7. #22
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,662
    Don't know, I could do it that way, too.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  8. #23
    Registered User
    Join Date
    May 2008
    Location
    Paris
    Posts
    248
    Don't know, I could do it that way, too.
    Only if I get your idea right: you want to create some sort of mapping between your parameter 'T' and one of the three integers in your struct? Then the typedef seems more appropriate.

  9. #24
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,662
    Well, as you could see in the code. If T = wchar_t, then I need wchar_t functions and wchar_t character constants. That's why I need this whole specialization mess in the first place.
    So it seems that your idea is right.
    Actually, when I think about it, I don't think it will work.
    They are string literals that I need, depending on the type of T:

    Code:
    	template<> const char* StrTraits<char>::UINT64_T_IDENTIFIER = "&#37;I64u";
    	template<> const char* StrTraits<char>::INT64_T_IDENTIFIER = "%I64i";
    	template<> const char* StrTraits<char>::LONG_IDENTIFIER = "%li";
    	template<> const wchar_t* StrTraits<wchar_t>::UINT64_T_IDENTIFIER = L"%I64u";
    	template<> const wchar_t* StrTraits<wchar_t>::INT64_T_IDENTIFIER = L"%I64i";
    	template<> const wchar_t* StrTraits<wchar_t>::LONG_IDENTIFIER = L"%li";
    Last edited by Elysia; 05-10-2008 at 06:30 AM.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  10. #25
    Registered User
    Join Date
    May 2008
    Location
    Paris
    Posts
    248
    Ok just for fun: you can also specify behavior of member functions / operators by simply calling functions in "behavior traits". These are usually called "policy classes".

    For example:

    Code:
    //general traits declaration
    template<> 
      struct StrTraits;
    
    // trait specialized
    template<UINT64_T_IDENTIFIER> 
      struct StrTraits
      {
        typedef UINT64_T_IDENTIFIER myIdentifier;
      };
    
    //policy for addition
    template<>
      struct StrAdd;
    
    template<UINT64_T_IDENTIFIER>
      struct StrAdd
      {
        static UINT64_T_IDENTIFIER add( UINT64_T_IDENTIFIER const& arg1 , UINT64_T_IDENTIFIER const& arg2);
      };
    
    template<typename T, 
                 typename traits = StrTraits<T>,
                 typename sum = SumPolicy<T> >
      class CTmplStringBase:
      {
      public:
       CTmplStringBase(const CStringT< StrTraitMFC_DLL<typename traits::myIdentifier> >& strSrc);
    
       friend
         typename traits::myIdentifier operator+( typename traits::myIdentifier const& arg1, typename traits::myIdentifier const& arg2) {
           return sum::add( arg1, arg2);
         }
      };
    The operator has been declared 'friend' for correspondence with the static function 'add' , you could also pass '*this' (don't forget the * ) as _first_ argument.

    Also notice that now you can only add two variables of the same type. Had I wanted to add variables of different types, I should have made a so-called "policy trait" : this is a mapping of type to function _and_ its return value. Example on request ;-)

  11. #26
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,662
    A trait class and a policy class. Huh... Looks interesting, but I don't think it will help very much on the linking errors >_<

    EDIT:
    Does anyone get linking errors like me? I'm at a loss on how to correct them...
    Attached Files Attached Files
    Last edited by Elysia; 05-10-2008 at 06:59 AM.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  12. #27
    Registered User
    Join Date
    May 2008
    Location
    Paris
    Posts
    248
    Oooops... I think I just violated one of Alexandrescus best coding practices:

    44. Prefer writing nonmember nonfriend functions

    http://www.ubookcase.com/book/Addiso...ch44.html#ch44

    so no friend, no member. There wasn't any why I made it friend, just a bad habit of what "they" have always told me to do.

  13. #28
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,662
    No need to worry about that. I hate non-member functions.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

  14. #29
    Registered User
    Join Date
    May 2008
    Location
    Paris
    Posts
    248
    I hate non-member functions.
    For what reason? I think Alexandrescu's argument is quite strong:

    "so that it will have the desirable property of accepting the same implicit conversions on its left-hand side and right-hand side parameters".

    A very nice way to implement operators is the following:

    http://www.ubookcase.com/book/Addiso...7lev1sec2.html

    This is a very useful book!

  15. #30
    C++まいる!Cをこわせ! Elysia's Avatar
    Join Date
    Oct 2007
    Posts
    22,662
    All it does for me is invoke the implicit operator instead of working as it should.
    Plus I'm not fan of "reversed" syntax.
    If (myobj == something) <--- Right
    if (something == myobj) <--- Wrong

    It may, of course, matter on circumstances, but I don't usually need the "reversed" syntax.
    But I especially hate non-member functions other than operators.
    Quote Originally Posted by Adak View Post
    io.h certainly IS included in some modern compilers. It is no longer part of the standard for C, but it is nevertheless, included in the very latest Pelles C versions.
    Quote Originally Posted by Salem View Post
    You mean it's included as a crutch to help ancient programmers limp along without them having to relearn too much.

    Outside of your DOS world, your header file is meaningless.

Page 2 of 5 FirstFirst 12345 LastLast
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Code Review for upcoming contest
    By Darryl in forum Contests Board
    Replies: 2
    Last Post: 02-28-2006, 01:39 PM
  2. Problem : Threads WILL NOT DIE!!
    By hanhao in forum C++ Programming
    Replies: 2
    Last Post: 04-16-2004, 01:37 PM
  3. True ASM vs. Fake ASM ????
    By DavidP in forum A Brief History of Cprogramming.com
    Replies: 7
    Last Post: 04-02-2003, 03:28 AM
  4. Interface Question
    By smog890 in forum C Programming
    Replies: 11
    Last Post: 06-03-2002, 05:06 PM
  5. Replies: 0
    Last Post: 02-21-2002, 05:05 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21