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

Returns pointer to FileInfo from ReadDir #77

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

the-plate
Copy link

Returns pointer to os.FileInfo from ReadDir() - same as Stat does. This would than ensure compatibility of return types for Stat() and ReadDir(). Now are different.

@ueffel
Copy link
Collaborator

ueffel commented Oct 18, 2023

Since the methods of the interface implementation do not have a pointer receiver shouldn't Stat() return the struct itself instead of a pointer to the struct?

@the-plate
Copy link
Author

What implementation do you mean exactly? I can see few having pointer receiver.

@ueffel
Copy link
Collaborator

ueffel commented Oct 18, 2023

https://github.com/studio-b12/gowebdav/blob/master/file.go
I mean the implementation of os.FileInfo by gowebdav.File

@the-plate
Copy link
Author

Ok I see.
On one hand, I'm bit confused, the File struct in file.go does not implement the mentioned Stat() function. And on the other hand, it it possible to call value receiver function on a pointer value. The pointer in such case is automatically dereferenced. On top of that the File implements only, say, getters so the pointer receiver is not necessary and value receiver can be used.

@the-plate
Copy link
Author

Hello, any comments on this topic ? Thanks!

@the-plate
Copy link
Author

Hi @ueffel could you please comment on this? Thanks

@ueffel
Copy link
Collaborator

ueffel commented Mar 5, 2024

Ah, I got my trail of thought again:

Returning *gowebdav.File (pointer to struct) as os.FileInfo (interface) has an unnecessary level of indirection. Returning a struct (gowebdav.File) as interface (os.FileInfo) already makes a pointer out of it.

To align the return types of ReadDir and Stat, Stat should be changed, not ReadDir:

 // Stat returns the file stats for a specified path
 func (c *Client) Stat(path string) (os.FileInfo, error) {
-       var f *File
+       var f File
        parse := func(resp interface{}) error {
                r := resp.(*response)
-               if p := getProps(r, "200"); p != nil && f == nil {
-                       f = new(File)
+               if p := getProps(r, "200"); p != nil {
+                       f = File{}
                        f.name = p.Name
                        f.path = path
                        f.etag = p.ETag

@chripo thoughts?

@chripo
Copy link
Member

chripo commented Mar 13, 2024

I'm think @ueffel is right with the unnecessary level of indirection.
Maybe we should refactor ReadDir to f = File{} too?

@ueffel
Copy link
Collaborator

ueffel commented Mar 13, 2024

Yeah, makes sense.

@the-plate
Copy link
Author

Thanks for comments, will you make the changes?

@chripo
Copy link
Member

chripo commented Mar 13, 2024

@the-plate feel free to make changes!

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.

None yet

3 participants