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

Double replacements cause unexpected results #189

Open
GoogleCodeExporter opened this issue Mar 24, 2015 · 12 comments
Open

Double replacements cause unexpected results #189

GoogleCodeExporter opened this issue Mar 24, 2015 · 12 comments

Comments

@GoogleCodeExporter
Copy link

I had the thought that maybe using a double dynamic subsitution (map x to y and 
map y to x) might cause an endless loop, so I wrote a test. The good news: no 
endless loop. The bad news: the result is not quite what I expected:

testDoubleReplaceEndlessLoop
    | result |
    self analyzer 
        when: [ :x | FLPair == x ] substituteBy: [ :x | FLWeakClassMock ];
        when: [ :x | FLWeakClassMock == x ] substituteBy: [ :x | FLPair ].

    result := self resultOfSerializeAndMaterialize: {FLPair. FLWeakClassMock}.

    self assert: result class equals: Array.
    self assert: result size equals: 2.
    self assert: result first equals: FLWeakClassMock. "this line will fail"
    self assert: result second equals: FLPair


"result" will be {FLPair. FLPair} instead of the expected {FLWeakClassMock. 
FLPair}

This is certainly not an important issue but we should at least decide if we 
want to do anything about it (maybe document the limit of dynamic substitution 
in this case)

Original issue reported on code.google.com by maxle...@gmail.com on 24 Feb 2013 at 2:55

@GoogleCodeExporter
Copy link
Author

Thanks Max

Original comment by tinchod...@gmail.com on 24 Feb 2013 at 6:25

@GoogleCodeExporter
Copy link
Author

HI max. I don't agreee with this one. I would expect  {FLPair. FLPair}.
All conditions block closures are evaluated (and in order). Maybe we should 
document this in the webpage?

Maybe we can add another message:  #when: ifNotAlreadyASubstituteSubstituteBy:
which checks if then object is already a subsitute does nothing and if not, 
performs the substitution?

Original comment by marianopeck on 24 Feb 2013 at 7:38

@GoogleCodeExporter
Copy link
Author

Hi Mariano, I understand. I don't have a clear position about this...

Original comment by tinchod...@gmail.com on 24 Feb 2013 at 8:16

@GoogleCodeExporter
Copy link
Author

Yes, I see your point. From a technical point of view this makes sense but it's 
confusing semantically. 

Personally, I figure that documenting this should be enough, it's an edge case 
anyhow. To implement Mariano's suggested method, there might be quite a few 
changes involved.

Original comment by maxle...@gmail.com on 24 Feb 2013 at 9:16

@GoogleCodeExporter
Copy link
Author

Sure it is that complicated?  Cannot be just send #isSubstitute: and do nothing 
when anserwing true?

Original comment by marianopeck on 25 Feb 2013 at 12:21

@GoogleCodeExporter
Copy link
Author

forgot to said isSubstitute: is already implemented

Original comment by marianopeck on 25 Feb 2013 at 12:21

@stale
Copy link

stale bot commented May 18, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will remain open but will probably not come into focus. If you still think this should receive some attention, leave a comment. Thank you for your contributions.

@stale stale bot added the stale label May 18, 2021
@theseion
Copy link
Owner

Should test this before closing.

@stale stale bot removed the stale label Oct 30, 2021
@stale
Copy link

stale bot commented Dec 29, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will remain open but will probably not come into focus. If you still think this should receive some attention, leave a comment. Thank you for your contributions.

@stale stale bot added the stale label Dec 29, 2021
@theseion
Copy link
Owner

theseion commented Jan 2, 2022

Not stale.

@stale stale bot removed the stale label Jan 2, 2022
@stale
Copy link

stale bot commented Mar 3, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will remain open but will probably not come into focus. If you still think this should receive some attention, leave a comment. Thank you for your contributions.

@stale stale bot added the stale label Mar 3, 2022
@theseion
Copy link
Owner

theseion commented Mar 4, 2022

Not stale

@theseion theseion added the pinned Never mark this issue stale label Mar 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants