LouisCharlesC / safe

Header only read/write wrapper for mutexes and locks.
MIT License
147 stars 11 forks source link

Make type names more intuitive / closer to the standard library #20

Open eyalroz opened 4 years ago

eyalroz commented 4 years ago

Consider playing with the names a little bit, to make them more reminiscent of the standard-library-only idiom, and/or more "narratively intuitive".

My main peeve here is with the names WriteAccess (from the overview) and its sibling ReadAccess. Those don't sound like class names; and the instantiation is about gaining write access, it's not that the instance is the write access.

Why not make it a little more like in the standard library, e.g.:

using safe_string = safe::safe<std::string>;
safe_string foo; 
safe_string bar;
std::string baz;
{
        safe::lock_guard lock(foo); 
    *foo = "Hello, World!";
}
std::cout << bar.unsafe() << std::endl; 

or maybe:

{
        auto lock = safe::lock_guard(foo);
    *foo = "Hello, World!";
}
LouisCharlesC commented 4 years ago

safe::lock_guard does not seem right because you need a way to specify whether it is a read-only or a read-write access. In similar libraries, I remember seeing read_lock and write_lock a lot. I do like these, but I guess I wanted safe not to look too much like the others!

I'm not sure I get your point about the

"instantiation is about gaining write access, it's not that the instance is the write access"

but I suspect it holds for names such as read_lock and write_lock.

From the code examples that you give, it seems you would like that the access to the value be done through the safe object, rather than the lock/access object:

safe::lock_guard lock(foo); *foo = "Hello, World!";

whereas the way the library works is like this:

safe::lock_guard lock(foo); *lock = "Hello, World!";

I never thought about this. I'll do now.

eyalroz commented 4 years ago

Second part:

I actually didn't even think about that conciously when I was writing the example. My fingers writing the code just assumed that if we have a safe string, we do stuff with/on the safe string.

First part:

safe::lock_guard does not seem right because you need a way to specify whether it is a read-only or a read-write access.

But it would be just like the standard: lock_guard is read-and-write. And for read-locking, you could have: safe::read::lock_guard(foo), or safe::lock_guard(foo, safe::read_access).

Alternatively, the two explicit options I mentioned for read-only access could be adapted for read-write access as well. Not a problem :-)

LouisCharlesC commented 4 years ago

I like it. Unfortunately, my eyes see safe::read::LockGuard(foo). Stupid CamelCase glasses...

eyalroz commented 4 years ago

Whoops, fixed a typo on one of my suggestions. If there's an extra param, no need for the sub-namespace.

LouisCharlesC commented 4 years ago

Not sure the extra param cuts it. Today, read-only Access and read-write Access are separate classes. The safe::lock_guard(foo, safe::read_access) you suggest could be a factory that returns the right object. But I do not know if that would fit every use case. Maybe sometimes you do need to call the constructor and not a factory.

I did start playing with the concept though, but did not find anything satisfying yet. I'll keep this open just as a reminder. If I ever implement such a change, I'll consider switching to snake_case. So I'll close #19 and only keep this issue alive.