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

Massive error wiping and re-architecturing #1

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

ekaitz-zarraga
Copy link

Hi,

I'm using this lib for a personal project and the internals are pretty much a broken mess 😞
I'm trying to fix the broken code and also some weird design decisions.

I open this PR as a WIP. I'll keep updating it if I have time.

Suggestions and improvements are welcome.

This way the compiler is the one that checks the language is supported,
we don't need to check it ourselves.
It has to be a cplusplus compiler to compile this file...
It's faster, safer, easier to read and maintain
TODO: do the same with all internal classes not only with HTTSDo
This uncovered many bugs:

- Ambiguous ifs
- Unexpected fallthroughs in switches

And many more!
It's possible that my fixes broke some stuff, as the algorithms are not
clear.

Also the formatting was so awful this changeset is really ugly. I'm
sorry.
@ekaitz-zarraga ekaitz-zarraga marked this pull request as draft December 4, 2023 00:41
@ekaitz-zarraga ekaitz-zarraga changed the title WIP: Massive error wiping and re-architecturing Massive error wiping and re-architecturing Dec 4, 2023
@ekaitz-zarraga
Copy link
Author

So the parameter control is just too convoluted, and it uses contradicting names (HDicDB vs HDicDBName).

I think the best is to remove all the get and set methods and use an htts_config class or something like that that is able to deal with all the possible configurations. Making strcmp to to bypass the compiler and cascade the configuration from there is a horrible idea.

Is there any chance to rewrite this in collaboration with you @aholab ?
I'd love to make this codebase more maintainable, but I lack the TTS context and I'd need some help organizing that in my mind to be able to rewrite this effectively.

Really cool project anyways!

C++ does not have a standard encoding for its files, if we write
literals with accents, they are considered UTF-8 by some compilers and
they get values that are higher than what a `char` can handle. That's
why there was an unneeded cast in this block.

Using the Latin1 encoding the code has defined, we can deal with all
this in `char` range, without errors. I don't really understand why
everything is full of characters with tildes, if there's a fantastic
`chset.h` that contains all the magic needed to avoid these issues and
be portable...
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

Successfully merging this pull request may close these issues.

1 participant