mbj / mutant

Automated code reviews via mutation testing - semantic code coverage.
Other
1.95k stars 153 forks source link

Advice on mutations of .freeze #449

Closed tjchambers closed 9 years ago

tjchambers commented 9 years ago

I took a look at mutant source code to see how mutant source itself uses .freeze/IceNine internally. AFAICT it is only freezing static text. And IceNine for more complex CONSTANTS.

In my code I have been using freezing for a couple elements:

1) TO reduce unnecessary object creation - especially at low levels internally 2) As one defensive technique to keep complex data structures that are conceptually immutable after creation from being mutated (bug detection)

For case 1 I have not been consistent in creating constants for strings, and I can do that rather than freeze in place. That of course is a better technique.

For case 2 I see no equivalents in mutant source, which I was hoping would guide me, as I am creating more complex arrays etc, that I feel should best be frozen after initialization - typically with IceNine.

Because this is done in private methods typically I can't directly add a spec indicating a var is to be frozen.

I could use some advice on how to utilize var freezing to achieve my objectives while preventing alive mutations. Shifting things to a predefined constant is not suitable. For vars publicly exposed I can directly add a spec, but for ones used internally without such visibility I am not clear how to proceed.

I am open to being told I am overdoing the protection through immutability, or over-thinking my technique, but my recent experience is I am finding places where inadvertent manipulation has been detected in this larger code base. Signally something to be immutable is a signal to the next developer to not plan on mutating it.

Any experience in this regard would be valuable.

mbj commented 9 years ago

2) As one defensive technique to keep complex data structures that are conceptually immutable after creation from being mutated (bug detection)

For case 2 I see no equivalents in mutant source, which I was hoping would guide me, as I am creating more complex arrays etc, that I feel should best be frozen after initialization - typically with IceNine.

We use an indirect strategy to archive case 2:

All these strategies produce a good (but apparently not perfect) backpressure to unintend value mutation.

As you asked this question, I'm thinking about writing a predicate that allows to verify a data structure is frozen. It likely would use the ice_nine generic data structure recursive enumerator, and would allow to write better specs. IceNine.is_deep_frozen?(some_stuff) etc. //cc @dkubb

I am open to being told I am overdoing the protection through immutability

I'll not do this. I consider immutable by default one of the most useful design patterns.

but my recent experience is I am finding places where inadvertent manipulation has been detected in this larger code base.

Same in my code bases.

Signally something to be immutable is a signal to the next developer to not plan on mutating it.

Yeah.

mbj commented 9 years ago

@tjchambers Please close this issue when you think you got your answer.

tjchambers commented 9 years ago

@mbj Thanks for these insights - I find them valuable. Let me consider them as they may elicit more questions.

tjchambers commented 9 years ago

In looking at the mutant code re: use of .deep_freeze it is used during constant creation rather than inline in methods, which side steps the mutant alterations I am experiencing during it's application in my style. Just a comment that in my case it logically creates an alive mutant. That is one aspect I am trying to overcome. 'IceNine.deep_freeze(nil)` does not raise errors. Not suggesting it should, but it leaves the mutation alive.

mbj commented 9 years ago

@tjchambers Can we close this issue?