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

Minor refactoring proposal #66

Closed
sfendourakis opened this issue Aug 9, 2023 · 2 comments
Closed

Minor refactoring proposal #66

sfendourakis opened this issue Aug 9, 2023 · 2 comments
Labels
enhancement New feature or request

Comments

@sfendourakis
Copy link

I was trying to understand the recognize_song function and I saw some possible code improvements. Please note that I'm new to this codebase and I might have gotten things really wrong, so feel free to correct me if I'm wrong!
https://github.com/dotX12/ShazamIO/blob/17627a805c5ac228b9dab42521591ac494001d6a/shazamio/api.py#L347-L353
Here I believe the logic behind the check for signature_generator.input_pending_processing is already included following code
https://github.com/dotX12/ShazamIO/blob/17627a805c5ac228b9dab42521591ac494001d6a/shazamio/algorithm.py#L87-L89
so my proposal would be

 song = await get_song(data=data) 
 audio = self.normalize_audio_data(song) 
 signature_generator = self.create_signature_generator(audio) 
 signature = signature_generator.get_next_signature() 
  
 if signature is None: 
     return {"matches": []} 

https://github.com/dotX12/ShazamIO/blob/17627a805c5ac228b9dab42521591ac494001d6a/shazamio/api.py#L355-L358
Here is will never go inside the while loop since signature_generator.get_next_signature() always returns a DecodedMessage() instance or None for the case we covered above. So my proposal would be

 results = await self.send_recognize_request(signature) 
 return results 

https://github.com/dotX12/ShazamIO/blob/17627a805c5ac228b9dab42521591ac494001d6a/shazamio/algorithm.py#L103-L108
This code is also in the __init__ method of the class so it's already applied, it seems to me that it's not needed here. My proposal would be to keep it only in the __init__ method.

If you believe there is any value in any of these suggestions I can open a PR, else please feel free to completely ignore.
Thanks in advance for your time

@dotX12
Copy link
Collaborator

dotX12 commented Aug 13, 2023

@sfendourakis, looks good, do a PR and I'll take a look at it :)
It is also desirable to add a couple of tests so that you can compare the old version and the new one. Because currently, test is only on one file.

@dotX12 dotX12 added the enhancement New feature or request label Aug 13, 2023
@dotX12
Copy link
Collaborator

dotX12 commented Feb 23, 2024

Better late than never, but it happened thanks to pyo3 and RUST.
https://github.com/dotX12/shazamio-core

Fixed in 0.5.0.
See comment: #76 (comment)

@dotX12 dotX12 closed this as completed Feb 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants