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

Add typed list #47 #48

Merged
merged 8 commits into from
Dec 6, 2019
Merged

Conversation

goofinator
Copy link
Contributor

Hello!
Added typed lists.
One type and tests for it per commit.
List now called from the StringList

@goofinator
Copy link
Contributor Author

I have a question. If i try to get FileList and one of the files opening fails. What is the best way to process it?
let's say: file1.txt nofile2.txt file3.txt
I'm processing it.
open file1.txt - success.
open nofile2.txt - failure.
I break the cycle with error immediately? if yes, should i close file1.txt?
I think both - yes.

@akamensky
Copy link
Owner

That’s a good question. Generally I think yes to both would be a good approach. Though it may not fit every use case, but would probably be fine to do just that.

For graceful closure of files - if we didn’t touch them it won’t be necessary as it won’t affect files anyhow. It only would become an issue if files were open for writing and the application wrote some bytes to it. So while is a good practice - may not be such a strict requirement I think.

@goofinator
Copy link
Contributor Author

If we go deeper (dicaprio.jpg) ... the situation of using several File / FileList arguments is unpleasant. If one of them fails to open, we get set of opened and not opened files. Of course, this will be a problem only if the program does not exit.

@goofinator
Copy link
Contributor Author

sorry about 66f217e and c4cc2e4. Didn't noticed how switched to the main branch.

@@ -511,13 +605,13 @@ func TestFileSimple1(t *testing.T) {
t.Errorf("Test %s failed with error: %s", t.Name(), err.Error())
return
}
defer file1.Close()

Copy link
Owner

Choose a reason for hiding this comment

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

Please leave 1 empty line between logical blocks for better readability. Too dense code is very hard to read

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

@akamensky akamensky merged commit 303bb40 into akamensky:master Dec 6, 2019
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