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

Why pointers instead of references? #36

Open
llem00n opened this issue Mar 26, 2023 · 2 comments
Open

Why pointers instead of references? #36

llem00n opened this issue Mar 26, 2023 · 2 comments

Comments

@llem00n
Copy link

llem00n commented Mar 26, 2023

One small question. What is the reason of using a pointer to a texture in, for example, LightingArea constructor? Because this way it feels really separated from the SFML design principles...

@MiguelMJ
Copy link
Owner

In the LightingArea class, the reason is to make the texture optional by allowing the pointer to be null.

When the texture is null, the area uses a plain color, as you can see here:
https://github.com/MiguelMJ/Candle/blob/master/src/LightingArea.cpp#L73

with some examples here
https://www.miguelmj.dev/Candle/md_doc_md_fog_ambiental.html

Today I'm not so sure whether I would write this again the same way, but if you ask me the reason for a pointer, there's why. Feel free to tell me here what alternative you would suggest.

Is there any other place where you feel the use of pointers is not justified?

@llem00n
Copy link
Author

llem00n commented Apr 4, 2023

Yeah, there is no problem in having a pointer to a texture as a private member of a class, it's the only option here.

But I was talking only about the interface of the classes.

I mean, look at the implementation of sf::Sprite, for example: sf::Sprite::m_texture

Under the hood it uses pointers, but the interface always takes references. Just because it looks more modern-c++-like and not c-like. There is nothing bad in writing in c-like style, but here we're talking about sfml-like style.

So, my issue was opened just because of that line: candle::LightingArea::LightingArea because I didn't see any point in sending a pointer here. If you don't have a texture --> use another constructor and that's all.

If you become interested in doing this, I can offer my help.

P.S. I saw in other issues you are really interested in keeping backward-compatibility, and I cannot agree more, it's really important. But sometimes, interface changes are not that bad, they can mean growth of the project and its maintainers in the way of programming.

These changes are not critical, but I'm sure will make the project more appealing. This does not mean, that we should forget about the backward-compatibility at all, there should be a note about breaking changes, and a version change (from 1.0.2 to 2.0.0). (check out the NOTE at the top of the readme and the number of stars of the repo ;) )

@MiguelMJ MiguelMJ mentioned this issue Sep 25, 2023
6 tasks
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