glassechidna / zxing-cpp

ZXing C++ Library
Apache License 2.0
598 stars 435 forks source link

CHARACTER_SET definition in .cpp, declaration in .h #39

Closed fschramka closed 7 years ago

fschramka commented 8 years ago

You need it somewhere in the project, so I just changed the definition. The last commit was just cause my formatter changed the whole file and the difference wasn't visible at once.

Hope it's alright :)

fschramka commented 8 years ago

doesnt work with vs2012 if I just declare and define it in the header file.

Benjamin-Dobell commented 8 years ago

@filipschramka What error do you get? I mean if all you do is delete the line:

const DecodeHintType DecodeHints::CHARACTER_SET;

in DecodeHints.cpp.

Leave everything else untouched from master.

So DecodeHints::CHARACTER_SET will still be defined and declared in the header.

fschramka commented 8 years ago

Fehler 1 error LNK2019: Verweis auf nicht aufgelöstes externes Symbol ""public: static unsigned int const zxing::DecodeHints::CHARACTER_SET" (?CHARACTER_SET@DecodeHints@zxing@@2IB)" in Funktion ""public: static class std::basic_string<char,struct std::char_traits,class std::allocator > __cdecl zxing::common::StringUtils::guessEncoding(char *,int,class std::map<unsigned int,class std::basic_string<char,struct std::char_traits,class std::allocator >,struct std::less,class std::allocator<struct std::pair<unsigned int const ,class std::basic_string<char,struct std::char_traits,class std::allocator > > > > const &)" (?guessEncoding@StringUtils@common@zxing@@SA?AV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@PADHABV?$map@IV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@U?$less@I@2@V?$allocator@U?$pair@$$CBIV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@@std@@@2@@5@@Z)". C:...\zxing-cpp-master\bin\libzxing.lib(StringUtils.obj)

fschramka commented 8 years ago

Sorry that it is in german^^ but it works fine, if you use my solution above :)

Benjamin-Dobell commented 8 years ago

Yeah, my German is pretty rusty, but Google translate to the rescue.

The point is you don't need all those changes. All you need to do is delete one single line, that's it. The value will still be declared and defined in the header, then you won't get any errors at all. I'm guessing the reason you got the error above about the unresolved symbol is because you deleted the line from the .cpp file but also deleted the definition in the header. Please do:

git remote add upstream https://github.com/glassechidna/zxing-cpp.git
git fetch upstream
git reset --hard upstream/master

Then go and delete the one line (in DecodeHints.cpp):

const DecodeHintType DecodeHints::CHARACTER_SET;

Then:

git add --update
git commit -m "Deleted duplicate CHARACTER_SET declaration"
git push -f origin master
fschramka commented 8 years ago

no I did not delete the definition in the header file...

Benjamin-Dobell commented 8 years ago

Hmm, I'm very confused. I just downloaded Visual Studio 2012 Express in a Windows 8.1 VM and compiled without any changes required.

fschramka commented 8 years ago

well thats the point, you can compile it without any changes. But if you create a new project and use the generated library, it won't work. Because you have multiple definitions, in your own file, where you include the DecodeHints.h file and inside the library.

Benjamin-Dobell commented 8 years ago

Sorry, to be clear, the whole solution builds fine. Both libzxing, but more importantly the zxing executable, which links to, and includes the headers for, libzxing exactly the same way your project should be using it. I even manually changed the compilation mode to be static (it's DLL by default), and didn't see any problems.

fschramka commented 8 years ago

Hmm yeah the solution works fine here as well... I build the whole solution with cmake (got a static lib then after compiling). The only thing I had to change before using zxing in my project was Multithreaded-Debug-DLL (/MDd) to Multithreaded-Debug (/MTd). But this shouldnt make any difference.

aidansteele commented 7 years ago

I think this is no longer needed, now that the build is passing. Let me know if that's not the case!