Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
eyalroz opened this issue Feb 29, 2020 · 5 comments
Open

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

eyalroz opened this issue Feb 29, 2020 · 5 comments

Comments

@eyalroz
Copy link

eyalroz commented Feb 29, 2020

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
Copy link
Owner

LouisCharlesC commented Mar 3, 2020

  • First part, naming:

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.

  • Second part, behavior:

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
Copy link
Author

eyalroz commented Mar 3, 2020

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
Copy link
Owner

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

@eyalroz
Copy link
Author

eyalroz commented Mar 3, 2020

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

@LouisCharlesC
Copy link
Owner

LouisCharlesC commented Mar 11, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants