tags:

views:

984

answers:

5

Background

I have a project named PersonLibrary which has two files.

  1. Person.h
  2. Person.cpp

This library produces a static library file. Another project is TestProject which uses the PersonLibrary (Added though project dependencies in VS008). Everything worked fine until I added a non-member function to Person.h. Person.h looks like

class Person
{
public:
    void SetName(const std::string name);

private:
    std::string personName_;
};

void SetPersonName(Person& person,const std::string name)
{
    person.SetName(name);
}

Person.cpp defines SetName function. When I try to use SetPersonName from TestProject, I get error LNK2005: already defined. Here is how I used it

#include "../PersonLibrary/Person.h"
int main(int argc, char* argv[])
{
    Person person;
    SetPersonName(person, "Bill");
    return 0;
}

Workarounds tried

1 - I have removed the Person.cpp and defined the whole class in Person.h. Error gone and everything worked.

2 - Changed the SetPersonName modifier to static. Like the below

static void SetPersonName(Person& person,const std::string name)
{
    person.SetName(name);
}

Questions

  1. Why the code shown first is not working as I expected?
  2. What difference static made here?
  3. What is the approapriate solution for this problem?

Thanks

+1  A: 
  1. The function SetPersonName will be compiled into each objectfile that includes the Person.h file, thus making the linker seeing several functions and giving the error.

  2. By writing static you state that the function will only be visible within a single objectfile. You will still get several functions in you binary but now you will not get the errors.

  3. Try to write inline before the function like

    inline void SetPersonName(Person& person,const std::string name)
    {
        person.SetName(name);
    }
    

    ...because the function is pretty simple it is OK I think to have it as an inline. An inline will place the necessary code where the function is used, without actually creating a function to be called.

epatel
+10  A: 

You either have to

  • move SetPersonName's definition to a .cpp file, compile and link to the resulting target
  • make SetPersonName inline

This is a well known case of One Definition Rule violation.

The static keyword makes the function's linkage internal i.e. available to the translation unit it is included in, only. This however is hiding the real problem. I'd suggest move the definition of the function to its own implementation file but keep the declaration in the header.

dirkgently
Thanks for the answer. I sorted it.
Appu
+1  A: 

By declaring the function static you are scoping it to the current translation unit, so in effect you have added a new SetPersonName function in your main file, and would be calling that not the one defined in the library.

The correct solution is to declare SetPersonName as extern in person.h and implement it in person.cpp

Person.h

extern void SetPersonName(Person& person,const std::string name);

Person.cpp

void SetPersonName(Person& person,const std::string name)
{
    person.SetName(name);
}
Rob Walker
A: 

A solution would be to make that function a static method. That will stop the "already defined" errors.

Geo
+1  A: 

When you compile you're library, its lib file contains a definition for SetPersonName. When you compile your program that uses the library, since it includes the header, and you've written the code inline in the header it also compiles in a definition for SetPersonName. Two definitions for the same function aren't (generally) allowed. The static keyword tells the compiler that the function shouldn't be exposed outside of the current translation unit (discrete piece of code you are compiling), so the definition in the library isn't visible to the linker.

The appropriate solution to this problem depends on your goals. Header files with static function declarations is almost never what you want. From a design standpoint I would recommend getting rid of SetPersonName altogether, and just use Person::SetName.

However, failing that, I would implement it much like you've done for the rest of your functionality, declarations in the header, and implementation in the .cpp. Inline functions associated with a library will tend to diminish many of the advantages of using a library in the first place.

Logan Capaldo