Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fixed Inconsistent return statements #467

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

GeneCodeSavvy
Copy link
Contributor

@GeneCodeSavvy GeneCodeSavvy commented Mar 6, 2025

#433 Refactored the code to comply with Pylint's recommendation.

if lines_type is bytes:
   # rdflib 5
   return b'\n'.join(lines) + b'\n'
elif lines_type is str:
   # rdflib 6
   return '\n'.join(lines) + '\n'
if lines_type is bytes:
   # rdflib 5
   return b'\n'.join(lines) + b'\n'
elif lines_type is str :
   # rdflib 6
   return '\n'.join(lines) + '\n'
else:
    return ''

elif lines_type is str:
# rdflib 6
return '\n'.join(lines) + '\n'
else:
return ''
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of returning an empty string I think this should be an error and we should raise an exception. Maybe a ValueError exception could be raised because lines is neither a bytes or a str.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. I've replaced the return empty string with raise ValueError('Lines must be either bytes or str')

@GeneCodeSavvy
Copy link
Contributor Author

GeneCodeSavvy commented Mar 7, 2025

I had to add changes corresponding to no-else-return to this PR, because the changes would have caused conflicts they were separate PRs.

@tcmitchell
Copy link
Collaborator

Unfortunately the combination of no-else-return along with inconsistent-return is very confusing. It would have been better to do one, then branch off of that to do another. I need to take a more detailed look at these changes. Please don't make any changes. It will take me some time to review.

@GeneCodeSavvy
Copy link
Contributor Author

I apologize, I will keep your recommendation in mind in future

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants