On Dec 3, 2019, at 6:49 AM, Ron Jeffries <ronjeffriesacm@...> wrote:
I hope this makes sense. I have a bad cold, and the old brain is not working well.
TL;DR: You gave me the idea that the non-mocking solution to the problem would be:
1. The `put_updatable_fields` function sets a field that says ¡°this particular structure is known to be updatable¡±.
2. We believe the value of that field corresponds to reality because of tests.
3. The tests for the N various functions that return updatable animals just check that field.
The disadvantage is that I¡¯d still have to arrange that reading an animal returns an animal that `put_updatable_fields` could handle, rather than a metaconstant. Not sure how I feel about the tradeoffs.
=======
It seems to me that this operation, read and make updatable (maybe form-compatible), would properly be a constructor.
Yes. I at one point had a variety of structures - they¡¯d be subclasses in a language with inheritance - for different animal use cases. That caused various problems that made me decide I wasn¡¯t going to rebel against what (I think) Ecto wants me to do.
I want a test, probably, for put_updatable_fields, and it needs to check all the fields. Since that takes and returns an animal, it can be tested with a canned empty animal and a canned full one and an irritating compare equal of some kind. But in mocks, I think it'd be much the same, since we have to check each field to see if it has been changed.
Yes. I wasn¡¯t explicit, but the two functions called in `updatable!` have to be tested in a non-mockish way.
I guess if I were mocking, then I'd just do the mock thing you show that says that updatable! does in fact call the put. Since I wouldn't likely be mocking (Detroit school, y'know), what would I do? I'd write a test that would fail if some should-be-updatable field wasn't updatable. (Which I really think means displayable or something.)
What I¡¯m now calling ¡°updatable¡± used to be ¡°showable¡±, but it turns out that what I really mean is ¡°the animal structure contains populated fields that can both be shown in a form and that, when changed, provoke an Ecto `update` to generate the right SQL to update/create the right table rows.¡± So ¡°updatable¡± was a better reminder.
I'm guessing that a non-updatable animal is still a valid animal. (That might be a bug.) It appears that updatable is a pure cosmetic thing to get the form to work, and that the animal has no reason to know whether it is updatable. I'm wondering why they aren't always made updatable out of the box. (Complete Constructor Method). If they were, would that change our thinking on how to do this? I think it might: we don't worry any more.
In effect, the four functions are Complete Constructors. What¡¯s really being tested is that the four Complete Constructors work correctly with the lower level `Read` code. That code can only fetch on-disk data; there has to be another layer of processing that fills in the fields that don¡¯t live on disk.
In a way, part of the issue here is that Ecto encourages you to have partially-constructed objects, anathema in the tradition you and I come from.
For example, when you read an `Animal`, you don¡¯t have to read the entire tree of ¡°included¡± (linked by a foreign key) structures. If you¡¯re reading an animal for a purpose that has nothing to do with ServiceGaps, you can avoid the join and data transfer of that table¡¯s data. I¡¯ve chosen not to worry about that - this is a small app - but you could imagine an app that needed more efficiency.
Anyway, now we're writing four functions which, I guess? could conceivably read animals wrongly, not calling updatable! and we want to be sure that they do?
Tell the truth, since this is a display issue (?), I think I'd just look.
Yes, though there are some complications here. (Forms don¡¯t actually display Animals, they display Changesets which can include error messages associated with the last attempt to process a field, and I find myself annoyingly prone to forgetting to add the HTML-generating code to show the error. Here, for example, is the code that annotates the `name` field with a message like `[name] is already taken¡±:
<%= text_input(f, :name) %> <%= error_tag f, :name %> <br/>
I forgot to add the `error_tag` for the service gap fields.
But what¡¯s needed here is self-discipline and manual testing, not error-prevention code. Probably.
If I worry that someone will try to make these four functions more efficient by removing the updatable bit, though, I need tests.
So either A) I'd have only one way to get an animal and it would be updatable, end of story I think, or
B1) write tests for each of those four functions. Do whatever is_updatable checking one does, one or many fields, etc.
B2) observe duplication and remove it. (def check_is_updatable(animal))?
B2 is the solution I would probably choose, see note at the top of the page.
