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

Finish chapter "Most of Spec in one example" #73

Closed
9 tasks done
koendehondt opened this issue Apr 19, 2024 · 3 comments
Closed
9 tasks done

Finish chapter "Most of Spec in one example" #73

koendehondt opened this issue Apr 19, 2024 · 3 comments
Assignees
Milestone

Comments

@koendehondt
Copy link
Collaborator

koendehondt commented Apr 19, 2024

Open issues from #52:

  • Shouldn't the section "An application manages icons" have an example? (No)
  • Figure "Better window" has a button "Save", but it has to be "Save Film". (Not applicable anymore due to the next item).
  • Figure "Better window" should show a window that is less high than the window in figure "Using the non homogeneous grid layout" because now it is unclear that the smaller initial intent has been applied. Actually, the figure should show a window, not a dialog because the dialog is introduced afterwards in section "Customizing the modal dialog".
  • There is a typo in the name field in figure "Customizing the dialog window". It reads "Chlinder". It should be "Schindler's List". I would write "1993" (the actual release year in full) instead of "98" in the field "Year".
  • The same typo is present in figure "Film list gets refreshed" and figure "Embedding the film description in the list: selecting a list item populates the detailed visual component".
  • In section "Adding more tests" probably there is a mistake in the last assert, which asserts on "name", while name has not been bound to another object since the previous assert. (There was no mistake)
  • In section "Using transmissions" there is a question from Stéphane. I think this section is too short for such an important feature.
  • Section "Styling the application" states that stylesheets are different for GTK and Morphic. That is sad because Spec is intended to be backend-independent. This cannot be fixed in the book. It is just an observation.
  • In the same section, there is a question from Stéphane that has to be addressed.
@koendehondt koendehondt added this to the Book release milestone Apr 19, 2024
@koendehondt koendehondt self-assigned this May 3, 2024
@koendehondt
Copy link
Collaborator Author

koendehondt commented May 3, 2024

  • A section about the dialog is missing between "Better looking FilmPresenter" and "Customizing the modal dialog". The latter refers to a dialog that has not been introduced.

@koendehondt
Copy link
Collaborator Author

In section "Using transmissions" there is a question from Stéphane. I think this section is too short for such an important feature.

The section is short because we have a dedicated chapter on transmissions.

The question from @Ducasse was:

""Esteban"" I do not understand why this is not working. I get an error with the ports.
""Enzo"" I think the error comes from the fact setModel: is called when the presenter is initialized and so there is no selection, thus a nil parameter. I managed to deal with it by adding a nil guard clause in setModel: but I wonder if we could make a "default selection" that would be the first film so that the presenter starts with a film to display details.

Making a default selection would maybe solve the problem in the example code, but it is not a general-purpose solution for the problem. When setting a model of a presenter, the presenter has to cover a possible nil value. I think that is important to state in the book.

So in my opnion, based on building a lot of UIs, the method setModel was implemented incorrectly because it did not take into account that the argument aFilm could be nil. I changed that method and now the code with the transmission works as expected. I added a few sentences to the text to explain why checking for nil is important.

@Ducasse
Copy link
Member

Ducasse commented May 4, 2024

Your analysis is correct!

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

No branches or pull requests

2 participants