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

Removed the extra close method. #141

Merged
merged 1 commit into from
Oct 14, 2024
Merged

Removed the extra close method. #141

merged 1 commit into from
Oct 14, 2024

Conversation

youpong
Copy link
Contributor

@youpong youpong commented Oct 12, 2024

Description

Removed the extra close method.

I felt that this might make the code a bit cleaner.
Note that the close method called the second time has no effect.

Cited for reference:

It is good practice to use the with keyword when dealing with file objects. The advantage is that the file is properly closed after its suite finishes, even if an exception is raised at some point.
-- Python Software Foundation. The Python Tutorial, Section 7.2: Reading and Writing Files

Related Issue(s)

None.

User-facing Changes

None.

Screenshots (If necessary)

None.

@youpong youpong requested a review from alec-kr as a code owner October 12, 2024 07:13
@alec-kr
Copy link
Owner

alec-kr commented Oct 14, 2024

I agree that removing the extra close() method makes the code cleaner and follows best practices for handling file objects. Using the with statement ensures the file is closed automatically, making the redundant close() call unnecessary.

Copy link
Owner

@alec-kr alec-kr left a comment

Choose a reason for hiding this comment

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

I'll go ahead and merge this change. Thanks again for helping improve the codebase!

@alec-kr alec-kr merged commit 67f0ada into alec-kr:main Oct 14, 2024
4 of 13 checks passed
@youpong youpong deleted the close-once branch October 14, 2024 11:01
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.

2 participants