Welcome, Guest. Please login or register. Did you miss your activation email?

Author Topic: nitpicking in Packet.cpp  (Read 2087 times)

0 Members and 1 Guest are viewing this topic.

FRex

  • Hero Member
  • *****
  • Posts: 1848
  • Back to C++ gamedev with SFML in May 2023
    • View Profile
    • Email
nitpicking in Packet.cpp
« on: August 05, 2014, 09:45:21 pm »
There are weird places, such as checking if sf::String length is more than 0 before iterating through it and adding characters (as if there is something to iterate through if length is 0) instead of just letting the iteration do 0 passes.
Taking size of std::string::value_type or char (which is always 1).
Sometimes (so not always, some functions do, some don't ie. <<std::string does, <<const char * doesn't) checking if length in bytes of data is 0 before passing it to append, while append ignores data of 0 length anyway.
Back to C++ gamedev with SFML in May 2023

Laurent

  • Administrator
  • Hero Member
  • *****
  • Posts: 32498
    • View Profile
    • SFML's website
    • Email
Re: nitpicking in Packet.cpp
« Reply #1 on: August 05, 2014, 11:16:07 pm »
Quote
checking if sf::String length is more than 0 before iterating through it and adding characters
I don't remember the reason for this check, it can probably be removed.

Quote
Taking size of std::string::value_type or char (which is always 1)
Just because it's 1 doesn't mean that we have to use 1. You could also say that sizeof(sf::Uint32) can be replaced with 4. There are several other overloads that deal with other data types, and most of them use sizeof. It just feels safer to use generic code rather than magic constants; and if one day some type changes for some reason, or if we copy and paste one function to add a new overload (more likely to happen), we won't have to check the code all over the place to replace these magic constants.

Quote
Sometimes (so not always, some functions do, some don't ie. <<std::string does, <<const char * doesn't) checking if length in bytes of data is 0 before passing it to append, while append ignores data of 0 length anyway.
Not sure, but std::string::c_str() might issue a warning in debug mode with some compilers if the string is empty.
Laurent Gomila - SFML developer

FRex

  • Hero Member
  • *****
  • Posts: 1848
  • Back to C++ gamedev with SFML in May 2023
    • View Profile
    • Email
Re: nitpicking in Packet.cpp
« Reply #2 on: August 06, 2014, 12:14:15 am »
Quote
It just feels safer to use generic code rather than magic constants
No constant at all is hardly 'magic'. You actually do that already: in >>std::string and >>char* you do just length bytes but in their corresponding operator<< you do length * sizeof.

Quote
we won't have to check the code all over the place to replace these magic constants.
You will have to replace the typenames inside sizeofs, since you have type names inside instead of dereferenced data pointer variable. So it's trading one 'magic' constant for another. Curiously you do do sizeof on variables instead of type names in single variable >> or << operators instead of consistently always naming type yourself OR always taking it from the compiler by using the variable itself.
Code for strings is hardly generic anyway, wchar_t is treated specially and made 32 bit then written char by char, sf::String is written char by char because endian and normal 1 byte char strings are passed as is.

Quote
Not sure, but std::string::c_str() might issue a warning in debug mode with some compilers if the string is empty.
This is really stupid of these compilers then but oh well. :(
« Last Edit: August 06, 2014, 12:18:30 am by FRex »
Back to C++ gamedev with SFML in May 2023