MiguelMJ / Candle

2D lighting for SFML
http://www.miguelmj.dev/Candle/
MIT License
130 stars 9 forks source link

Why pointers instead of references? #36

Open llem00n opened 1 year ago

llem00n commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 ;) )