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

Improve consistency in StandardMultipartFile implementation #34241

Conversation

nuheajiohc
Copy link

This PR refactors the StandardMultipartFile class to enhance consistency

Changes include:

  1. Removed the transferTo(Path dest) method from StandardMultipartFile, as it is already provided as a default method in the MultipartFile interface.
  2. Updated the transferTo(File dest) method to use getInputStream() for consistency with the MultipartFile interface's default transferTo(Path dest) implementation.
  3. Modified the isEmpty and getBytes methods to also utilize internal methods (getSize() and getInputStream() respectively) for better code consistency.

@nuheajiohc nuheajiohc force-pushed the refactor/StandardMultipartFile branch from 772a605 to d6fb4a3 Compare January 11, 2025 05:28
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 11, 2025
@nuheajiohc nuheajiohc force-pushed the refactor/StandardMultipartFile branch from ad95a01 to 1fcc63e Compare January 11, 2025 07:41
@snicoll
Copy link
Member

snicoll commented Jan 14, 2025

Thanks for the PR but I think the code is fine as it is. The internal methods call what is expected, irrespective of the fact they implement a public contract as well. For instance, we may return a different InputStream than the one we'd be using internally to copy to a byte array.

There are other places in the codebase where we do this. If we decided to change that, then we should be doing that consistently, not in an isolated class.

@snicoll snicoll closed this Jan 14, 2025
@snicoll snicoll added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants