bcrypt.net icon indicating copy to clipboard operation
bcrypt.net copied to clipboard

Fix the namespace/class issue

Open MisinformedDNA opened this issue 2 years ago • 5 comments

As evidenced by the numerous issues here and in SO, there is much confusion and frustration with the namespace and class being named the same thing. (https://github.com/BcryptNet/bcrypt.net/issues/8, https://github.com/BcryptNet/bcrypt.net/issues/11, https://github.com/BcryptNet/bcrypt.net/issues/51, https://github.com/BcryptNet/bcrypt.net/issues/99, https://stackoverflow.com/questions/31844709/conflicting-namespace-resolution). This was a mistake from the very beginning and it was never fixed.

Yes, it's breaking changes and no one likes breaking changes. But a mistake was made that violates best practices and guidance and sometimes the break is worth the fix. Every new user of this library has to go through the same head scratching/smacking/pounding and losses valuable time.

Let's make things better for develops and fix this issue in the next major version of BCrypt. I propose the namespace be changed to BCryptNet. This is a simple search and replace for the library owners and users.

It doesn't even need to be a breaking changes. We could just copy the current code into a new namespace and deprecate the classes in the old one. For this, we wouldn't even need to wait for the next major version.

MisinformedDNA avatar May 27 '22 17:05 MisinformedDNA

Not a huge fan of breaking existing setup; in essence I inherited the issue and .net kindly added static imports to make it less of an issue...

using static BCrypt.Net.BCrypt;

Some of the issue comes from changes over time declaring usage inside namespaces was never an issue https://github.com/BcryptNet/bcrypt.net/blob/main/IntegrationTest/TestVariousVersionsOfDotNet/DotNet2/Program.cs

Outside and ... Urgh.

I considered it early on but the aim was to reduce churn switching libs.

ChrisMcKee avatar May 28 '22 09:05 ChrisMcKee

Copying the namespace instead of moving it requires no breaking change.

MisinformedDNA avatar May 31 '22 14:05 MisinformedDNA

I cant get the example from the readme to compile, and I think the problem is related to the namespace issue mentioned here.

BCrypt.Verify("my password", "passwordHash");

Error:

The type or namespace name 'Verify' does not exist in the namespace 'BCrypt' (are you missing an assembly reference?)

This is the error I see regardless of whether I import with:

  • using BCrypt.Net;
  • using static BCrypt.Net.BCrypt;
  • Both of these
  • None of these

What does work is to use the second option, using static BCrypt.Net.BCrypt;, and just call the Verify method:

using static BCrypt.Net.BCrypt;
Verify("my password", "passwordHash");

Or forget about the namespace import and just fully qualify the method call:

BCrypt.Net.BCrypt.Verify("my password", "passwordHash");

So I'm really confused about the example in the readme - how would you suggest getting that to work?

devklick avatar Aug 20 '22 11:08 devklick

@devklick its an inherited namespace issue; You can either do as you have done or beyond that it depends on where your using statements sit. Outside of namespace it causes issues; within namespace xxx { using... } it was fine. Obviously this is more of an annoyance in Net6 with global usings etc.

I'll be renaming it in V5 and leaving a skeleton of the original namespace/class marked obsolete.

ChrisMcKee avatar Aug 22 '22 10:08 ChrisMcKee

I cant get the example from the readme to compile, and I think the problem is related to the namespace issue mentioned here.

BCrypt.Verify("my password", "passwordHash");

So I'm really confused about the example in the readme - how would you suggest getting that to work?

I've added a few more examples to the readme as its a common one. Obviously most will sink into oblivion with a rename; but it should help current users.

The addition:


DotNet4/6 etc

Due to the naming of the library if the namespace is after the using statements the call changes as .net fails to resolve naming correctly I'd advise rather than enter the entire namespace you simply use the import aliasing as below.


using BC = BCrypt.Net.BCrypt;

namespace DotNetSix;

string passwordHash =  BC.HashPassword("my password");

DotNet6

You can also do aliasing at the CSProj level; and wouldn't need to add the using statement at all

This example would allow you to use an alias BC.HashPassword() in your code

    <ItemGroup>
        <!-- emits global using BcryptNet = global::BCrypt.Net.BCrypt; -->
        <Using Include="BCrypt.Net.BCrypt" Alias="BC"/>
    </ItemGroup>

This version would allow you to just call Verify and HashPassword in your code base without any other reference.

    <ItemGroup>
        <!-- emits global using static global::BCrypt.Net.BCrypt; -->
        <Using Include="BCrypt.Net.BCrypt" Static="True"/>
    </ItemGroup>

ChrisMcKee avatar Sep 12 '22 12:09 ChrisMcKee