Error encapsulation

[Lewiscowles1986]

Part 1 of addressing #2011

The exercise that follows this document asks to encapsulate one error, but not any others.
Both the knowledge of how to encapsulate, as well as that it is a requirement seemed absent to me.

[github-actions]

Dear Lewiscowles1986

Thank you for contributing to the Go track on Exercism! 💙
You will see some automated feedback below 🤖. It would be great if you can make sure your PR covers those points. This will save your reviewer some time and your change can be merged quicker.

• 📜 The following files usually contain very similar content.

  • concepts/<concept>/about.md
  • concepts/<concept>/introduction.md
  • exercises/concept/<exercise>/.docs/introduction.md

  Please check whether the changes you made to one of these also need to be applied to the others.

• ✍️ If your PR is not related to an existing issue (and is not self-explaining like a typo fix), please make sure the description explains why the change you made is necessary.

• 🔤 If your PR fixes an easy to identify typo, if would be great if you could check for that typo in the whole repo. For example, if you found Unicdoe, use "replace all" in your editor (or command line magic) to fix it consistently.

Dear Reviewer/Maintainer

• 📏 Make sure you set the appropriate x:size label for the PR. (This also works after merging, in case you forgot about it.)

• 🔍 Don't be too nit-picky. If the PR is a clear improvement compared to the status quo, it should be approved as clear signal this is good to be merged even if the minor comments you might have are not addressed by the contributor. Further improvement ideas can be captured in issues (if important enough) and implemented via additional PRs.

• 🤔 After reviewing the diff in the "Files changed" section, take a moment to think about whether there are changes missing from the diff. Does something need to be adjusted in other places so the code or content stays consistent?

_{Automated comment created by PR Commenter 🤖.}

[Lewiscowles1986]

If acceptable to exercism; I can squash the commits. I was using exercism on the web and decided to stay in the web, where I'm not aware of squash or commit amend functionality.

[andrerfcsantos]

Hi,

Thanks for the PR.

In my opinion, this PR doesn't add much, and I'm having some issues trying to see what a student might get from this. My first issue is with the term "encapsulation". For me, encapsulation means hiding implementation details by only exposing behavior, which ideally is detached from a concrete implementation and I don't see how it applies here.

I find the description to be a bit vague by not providing a concrete example on how this can be useful. The description tries to sell the idea that we must check for a particular error and do something else with it other than returning it. My question when reading this is: ok, so what should we do instead? By not providing a concrete example of that "something else" we should do with the error, In the example from the "Checking for Errors" section of this file, we already provide an example on how to do something else when the error is of a particular error and I'm failing to see how this is different.

I think you are trying to introduce the student to the concept that we can check for certain error types and do something different with them based on its type, and I think that's why you are comparing them to exceptions. However, this topic is probably a better fit for the Error Wrapping concept where we can properly introduce the student to error type checking.

If acceptable to exercism; I can squash the commits. I was using exercism on the web and decided to stay in the web, where I'm not aware of squash or commit amend functionality.

In your branch, it's personal choice whether you create several commits or squash/amend them to make sure it all fits into just one. When merging, we squash all the commits, so all PR's end up being just 1 commit regardless.

[Lewiscowles1986]

Hi.

Thanks for taking the time to review and give such volumous feedback.

trying to see what a student might get from this

The wish/intent is for a student to understand a thing they will be asked to do in the task which covers errors as a concept, which I believe was not explained enough for new learners. In that task, a specific type of error is encapsulated and hidden within the task; which does not mention to forward all errors, but one; but readily requires it as far as I can see.

I find the description to be a bit vague by not providing a concrete example on how this can be useful.

My apologies for failing to capture the use of this. My attempt was to give enough information, without doing the work for the learner. As I have before, and I am sure I will again, I clearly failed to convey this:

The purpose might be to differentiate between things in-scope of a library or framework; and things that have not (perhaps yet) been considered; or are deliberately excluded from a domain or context.

To my mind, the only reasons not to forward an error to a user, are to:

• Substitute it for your own internal error. Perhaps raising a socket closed to be a HTTP error in a library encapsulating the complexities of low-level sockets. (I did not want to give such a concrete example of this, as it's not why the error is handled in a special-case in the follow-up exercise.
• Define behavior, which is appropriate to a specific domain, or context.

I did not wrap the error, by passing it to the error format using %w, because the task does not demonstrate this, or require it.

Can you think of other reasons this is a useful pattern? I Too am always trying to learn new things 😉.

In any case. I Can accept this isn't what is needed, but I might need some help to improve it. Alternatively if nobody has time, I am happy to close this, as I accept from your comments; it's not quite enough, but don't think I can think of a clearer, more concise explanation; than I've provided.

Best

[junedev]

Just as @andrerfcsantos, I also think "Encapsulation" and the coding pattern shown in this PR are two separate concerns. For example, in the following code, there is no encapsulation happening.

var ErrNotFound = errors.New("not found")

// ...

if err != nil && err != ErrNotFound {
  // e.g. write a 500 status code for an http response
}

Encapsulation should be discussed in the error wrapping concept.

Putting that aside, the other question remains whether or not we want to present the pattern of "returning for all but one specific error" (if err != nil && err != ErrSomethingSpecific) in the introduction. This pattern avoids nested if statements in the error handling in the case of this specific exercise (see examplar solution vs common community solution). My feeling is that this pattern is not very common as usually you can just handle the specific errors first and handle the "rest of the errors" last. It is only in our very specific case in this exercise that this does not work as we don't want to return in the case of the ErrScaleMalfunction but continue with the program execution. (Which leads us to yet another questions: Is it a good idea that the exercise requires the student to use a result even though there was an error?)

So I would vote for closing this PR and having a discussion about changing the exercise so that the student does not have to work with a result of a function even though there was an error. (I will leave a comment about this in #2011.)

[andrerfcsantos]

The wish/intent is for a student to understand a thing they will be asked to do in the task which covers errors as a concept

@Lewiscowles1986 I see. Then you are right in making a PR like this one, as the introduction must indeed have everything the student needs to solve the exercise.

However, like @junedev suggested in #2011, I also agree that instead of adding this to the description, we should just change the exercise so it doesn't need this.

For this reason I'll close this PR and forward further discussion to #2011.

Many thanks @Lewiscowles1986 for bringing this to our attention!