Refactor: BLF API Function Overloading #120029

Harley Acheson · 2024-03-28T18:52:59+01:00

Harley Acheson commented

2024-03-28 18:52:59 +01:00

Simplify some BLF functions by using overloading where possible. For
example "BLF_load" with two signatures for loading from files versus
memory. One "BLF_display_name" takes a name, the other an ID.
"BLF_unload" that works with filenames or IDs, etc.

Simplify some BLF functions by using overloading where possible. For example "BLF_load" with two signatures for loading from files versus memory. One "BLF_display_name" takes a name, the other an ID. "BLF_unload" that works with filenames or IDs, etc.

Harley Acheson added 1 commit 2024-03-28 18:53:09 +01:00

bac19eb83a Refactor: BLF API Function Overloading

Simplify some BLF functions by using overloading where possible. For
example "BLF_load" with two signatures for loading from files versus
memory. One "BLF_display_name" takes a name, the other an ID.
"BLF_unload" that works with filenames or IDs, etc.

Harley Acheson requested review from Hans Goudey 2024-03-28 18:53:21 +01:00

Hans Goudey reviewed 2024-03-28 19:01:31 +01:00

Hans Goudey left a comment

Sorry to be negative, but I'm not really sure this an improvement. To me it looks like the code is losing detail that's helpful to the reader like "this function is expecting a file" or "this function wants raw memory rather than an id".

This is so subjective, and if it was new code maybe I wouldn't question it-- I wouldn't be surprised if people's preferences aren't completely self consistent. But I don't see an obvious improvement here honestly.

Sorry to be negative, but I'm not really sure this an improvement. To me it looks like the code is losing detail that's helpful to the reader like "this function is expecting a file" or "this function wants raw memory rather than an id". This is so subjective, and if it was new code maybe I wouldn't question it-- I wouldn't be surprised if people's preferences aren't completely self consistent. But I don't see an obvious improvement here honestly.

Harley Acheson commented

2024-03-28 19:05:30 +01:00

Sorry to be negative...

Don't be sorry at all. I honestly had no idea if this would be considered an improvement at all.

There are times when I wished we had overloading, like with rcti/rctf or color conversions maybe? But I certainly understand your point. But I had this diff laying around so I thought this would be a great way to ask about it.

Thanks!

> Sorry to be negative... Don't be sorry at all. I honestly had no idea if this would be considered an improvement at all. There are times when I wished we had overloading, like with rcti/rctf or color conversions maybe? But I certainly understand your point. But I had this diff laying around so I thought this would be a great way to ask about it. Thanks!

Harley Acheson closed this pull request

2024-03-28 19:05:32 +01:00

Hans Goudey commented

2024-03-28 19:15:10 +01:00

Yeah, there are definitely lots of places where overloading is helpful. Practically required even! But maybe it's more useful when it's the same operation on different types. Or maybe the trouble is the lack of semantically meaningful types in this whole API (i.e. FontFile and FontID might be much more meaningful than const char * and int).

Yeah, there are definitely lots of places where overloading is helpful. Practically required even! But maybe it's more useful when it's the same operation on different types. Or maybe the trouble is the lack of semantically meaningful types in this whole API (i.e. `FontFile` and `FontID` might be much more meaningful than `const char *` and `int`).

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Refactor: BLF API Function Overloading #120029

Pull request closed