Page 1 of 1

Feedback

Posted: 25 Apr 2013, 16:16
by brunoma
Was just doing a preliminary scan through the API to see if we can convert our current C++ to C# / Objective-C / Java SmartFox bridge to using the new C++ api, and have the following thoughts:

Can you update your API to be const-correct?

In regards to the API, why did you decide to pass everything(std::string mostly) as pointers as opposed to ref's (const ref's)?
I think this makes the usage of the API a bit less clear and introduces the possibility of programmer error

Any plans for a VS2008 project?

Does this lib depend on C++11? ( I noticed a std::map::at is used in a couple places and was having trouble getting the boost stuff to build with the older msvc-9.0 toolchain )

What is the rationale behind the StringFormatter class as opposed to more conventional C++ methods?

If using boost asio for networking, why not use it for delegates as well?

Re: Feedback

Posted: 26 Apr 2013, 11:40
by MBagnati
Thanks for your comments.

You say: "Can you update your API to be const-correct?"
What do you mean?
You think that should be better declare as "const" the pointers exchanged between API and client application (or between API functions) whenever pointers do not need to be changed?
If this is your suggestion, I think that it is a good idea

You say: "why did you decide to pass everything(std::string mostly) as pointers as opposed to ref's (const ref's)?"
I am agree that a reference is usually preferred over a pointer whenever you don't need to reseat it.
For instance I think that references are useful in a class's public interface...or in other words I think that references should appear on the skin of an object, and pointers on the inside.
In C++ API we have choosen the usage of pointers instead of references basically for five reasons:
  • Try to have only one type of entities between API and client application: all are pointers or all are references
  • Pointers can be directly set to NULL while references cannot (a reference can only be a reference to a null pointer, but it cannot be NULL).
    The NULL value is useful to indicate the absence of a value for instance in "Get" methods to return the information that required item is not found...or to provide "empty" parameters calling a function
  • We can have pointers to a void type but references to void are forbidden. Void entities are used internally to store instances of SFS objects when is not important preserve the entity type.
  • Pointers can be used to return the address of a new-allocated entity. Sometimes API returns to client application the memory location of a new allocated entity
  • A reference must be initialized when it is declared; in fact it is an "alias" to an existing memory-entity.
    Everywhere software needs to store a pointer to an object in order to use it later, I think that we cannot store references, but we must store pointers....References (for instance a reference can be a member of a class), need to be initialized in the member list for all of the constructors

You say: "Any plans for a VS2008 project?"
Today we have compiled API with VS2010 and VS2012.
We have selected the last version of Microsoft IDE and the previous one for who has not already updated its working environment.
Currently we have not planned to compile API with older Visual Studio versions but on the other hand we have written the API using standard C++ so should be easy move to VS2008, if you need.

You say: "Does this lib depend on C++11?"
API does not depend on C++11.
We have written API using standard C++.
References to C++11 are bugs and we can fix them.

You say: "What is the rationale behind the StringFormatter class as opposed to more conventional C++ methods?"
StringFormatter is simply a wrapper of C++ sprintf that only resize the target string to be sure that exist the required space to concatenate the parameter(s) to add.

You say: "If using boost asio for networking, why not use it for delegates as well?"
Thanks for this suggestion.
There is not a real reason to prefer a self-made delegate (as actually used in API) to a boost delegate.
We will check the possibility to move to boost delegates.
In the past when API development was began, we have been the idea to limit the usage of BoostAsio to parts strictly linked to operating system (threads, network layer, timers)
using a self-made solution to grant the portability on all platforms for all other parts.
For this reason BoostAsio is currently used only for some aspects (like network layer) while others parts (like delegates) are not based on this library.

Re: Feedback

Posted: 26 Apr 2013, 16:45
by brunoma
In regards to const-correctness, yes marking variables that will not change as const will make your API easier to read and understand as well as integrate with other people's code that is const-correct:

http://en.wikipedia.org/wiki/Const-correctness

In regards to pointer usage, you should try to eliminate pointer usage as much as possible as this is a major source of potential errors. The reasons that you list for not using refs are precisely the reasons why you should be using them. The way you are using them also makes it a pain to use the API.

For example,

This is what i want to do:

Code: Select all


sfs->Connect("localhost");

This is what you are making me do:

Code: Select all

std::string server("localhost");
sfs->Connect(&server);
or

Code: Select all

std::string* server = new std::string("localhost");
sfs->Connect(server);
delete server;

Now if you are doing something in your API code like copying that pointer, you are likely to just copy the pointer address instead of copying the string,
leading to potential bad memory accesses(like, SmartFox::lastIpAddress...). "Are you going to be keeping my pointer? When am I allowed to delete it?"

Also I would suggest using boost's shared pointer or rolling your own reference counting for any entities created by smartfox that get handed off to the client using the smartfox c++ lib
"Are you going to delete this pointer or am I?"

Re: Feedback

Posted: 30 Apr 2013, 08:45
by MBagnati
I think that your suggestions (for instance the usage of const) are good ideas to make API more robust.
I also think that you are right when ask a major clearness about the usage of pointers and about their allocation/release.
I will try to apply your hints in API code.
Thanks for your suggestions.