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

Suggestion: Rename RWops types and functions to make more sense #7455

Closed
flowCRANE opened this issue Mar 13, 2023 · 4 comments
Closed

Suggestion: Rename RWops types and functions to make more sense #7455

flowCRANE opened this issue Mar 13, 2023 · 4 comments
Milestone

Comments

@flowCRANE
Copy link

flowCRANE commented Mar 13, 2023

From the beginning, I don't like the name RWops, because it's a bunch of letters, which at first glance you don't know what they mean. Also, the formatting of these names does not follow the naming convention used in the rest of the library — RwOps would be more appropriate.

Since read/write operations are about reading and writing data to any stream, why not just call it SDL_Stream? If you do this, the clarity will be high, the purpose of the functions will be clearly visible. The functions would rename to:

SDL_AllocRWSDL_AllocStream
SDL_FreeRWSDL_FreeStream
SDL_RWcloseSDL_StreamClose
SDL_RWFromConstMemSDL_StreamFromConstMem
SDL_RWFromFileSDL_StreamFromFile
SDL_RWFromFPSDL_StreamFromFP
SDL_RWFromMemSDL_StreamFromMem
SDL_RWreadSDL_StreamRead
SDL_RWseekSDL_StreamSeek
SDL_RWtellSDL_StreamTell
SDL_RWwriteSDL_StreamWrite

What do you think about it? This issue is related to the #6569.

@slouken slouken added this to the 3.2.0 milestone Mar 13, 2023
@slouken
Copy link
Collaborator

slouken commented Mar 13, 2023

There are lots of kinds of streams, so I'd prefer something like IOStream, but @libsdl-org/a-team thoughts?

@flowCRANE
Copy link
Author

There is a base class in the Object Pascal standard library, which is a similar abstraction to RWops. Any other streams inherit from it, including streams for files, arbitrary memory buffers, strings etc.. Its name is just TStream (Free Pascal, Delphi) — hence my suggestion to replace RWops simply with Stream since it also applies to arbitrary streams and data buffers.

I like your proposal — IOStream is clear and unambiguous.

@1bsyl
Copy link
Contributor

1bsyl commented Mar 16, 2023

"RWops" also has the meaning of "this can be overloaded", e.g. calling SDL_CreateRW like https://github.com/libsdl-org/SDL/blob/main/docs/README-migration.md#sdl_rwopsh to re-implement SDL_RWFromFP()

(There is also SDL_AudioStream which is some king of IO. I thought it would be elegant to create AudioStream on top of RWops.
but OTHO, as soon as you'll try to grep/find to a stream function, you'll end up with much more occurrence. so let's not change that)

In short, I'm fine with the current name / don't mind much if that changes.

@slouken
Copy link
Collaborator

slouken commented Nov 8, 2023

Let's go ahead and leave it as-is for now. @libsdl-org/a-team, feel free to weigh in if you like a different naming for these functions.

@slouken slouken closed this as not planned Won't fix, can't repro, duplicate, stale Nov 8, 2023
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

3 participants