[Pharo-project] Why StringHolder
Hi! Related with the issue http://code.google.com/p/pharo/issues/detail?id=2757(Issue 2757 <http://code.google.com/p/pharo/issues/detail?id=2757>: Refactor StringHolder) I was looking at the StringHolder hierarchy, and well, it's not very nice. Playing to see how difficult should it be to refactor the hierarchy and decouple a bit those things, I took Workspace off from there, and writing 2 methods it happened to work well (at least with what I tested, hehe). Sooo, I wondered why a StringHolder, what responsibilities it has, and what ones should it have... I found that most of the messages a StringHolder understands are for browsing things and inspect things, such as the Paragraph editor does :). And that's why Workspace could do all his stuff when I made him inherit from Object... StringHolder class comment says: "I am a kind of Model that includes a piece of text. In some cases, the text can be edited, and in some the text is a method." And I smell that its main responsibility is to hold a Text or a String in its contents variable, and all its subclasses are there because of that... And if so, I don't like that inheritance approach :). I want to destroy (literally) that hierarchy :). And maybe it helps us in coming refactors and cleanings, so we can do them with less fear :P. Well, this mail is just for ask for advice and argue about the correctness of this change (or not). Thanks for reading up to here! Guille
Benjamin started and rewrote the recentMessages and friends. If people can review his code it would be great.
Related with the issue http://code.google.com/p/pharo/issues/detail?id=2757 (Issue 2757: Refactor StringHolder) I was looking at the StringHolder hierarchy, and well, it's not very nice. Playing to see how difficult should it be to refactor the hierarchy and decouple a bit those things, I took Workspace off from there, and writing 2 methods it happened to work well (at least with what I tested, hehe).
Sooo, I wondered why a StringHolder, what responsibilities it has, and what ones should it have... I found that most of the messages a StringHolder understands are for browsing things and inspect things, such as the Paragraph editor does :). And that's why Workspace could do all his stuff when I made him inherit from Object...
I think that we should - Replace ParagraphEditor by SmalltalkEditor, TextEditor (did not check but it should be like that in CUIS and Squeak) In Squeak TextMorph >> initialize "TextMorph initialize" "Initialize the default text editor class to use" DefaultEditorClass := SmalltalkEditor. - then we should probably rewrite some parts and do not inherit from StringHolder I never understood why MessageSet is a subclass of Browser :) But I would focus on the
StringHolder class comment says: "I am a kind of Model that includes a piece of text. In some cases, the text can be edited, and in some the text is a method."
And I smell that its main responsibility is to hold a Text or a String in its contents variable, and all its subclasses are there because of that... And if so, I don't like that inheritance approach :).
This is exact.
I want to destroy (literally) that hierarchy :). And maybe it helps us in coming refactors and cleanings, so we can do them with less fear :P.
So could you review benjamin code? then check the texteditor/paragraphEditor
Well, this mail is just for ask for advice and argue about the correctness of this change (or not).
Thanks for reading up to here! Guille _______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
On Tue, Aug 10, 2010 at 6:00 AM, Stéphane Ducasse <stephane.ducasse@inria.fr
wrote:
Benjamin started and rewrote the recentMessages and friends. If people can review his code it would be great.
Ok, I'll watch it a few minutes this evening.
Related with the issue http://code.google.com/p/pharo/issues/detail?id=2757 (Issue 2757: Refactor StringHolder) I was looking at the StringHolder hierarchy, and well, it's not very nice. Playing to see how difficult should it be to refactor the hierarchy and decouple a bit those things, I took Workspace off from there, and writing 2 methods it happened to work well (at least with what I tested, hehe).
Sooo, I wondered why a StringHolder, what responsibilities it has, and what ones should it have... I found that most of the messages a StringHolder understands are for browsing things and inspect things, such as the Paragraph editor does :). And that's why Workspace could do all his stuff when I made him inherit from Object...
I think that we should - Replace ParagraphEditor by SmalltalkEditor, TextEditor (did not check but it should be like that in CUIS and Squeak)
What's CUIS? :(
In Squeak
TextMorph >> initialize "TextMorph initialize" "Initialize the default text editor class to use" DefaultEditorClass := SmalltalkEditor.
- then we should probably rewrite some parts and do not inherit from StringHolder I never understood why MessageSet is a subclass of Browser :) But I would focus on the
on the ...?
StringHolder class comment says: "I am a kind of Model that includes a piece of text. In some cases, the text can be edited, and in some the text is a method."
And I smell that its main responsibility is to hold a Text or a String in its contents variable, and all its subclasses are there because of that... And if so, I don't like that inheritance approach :).
This is exact.
I want to destroy (literally) that hierarchy :). And maybe it helps us in coming refactors and cleanings, so we can do them with less fear :P.
So could you review benjamin code?
This evening I'll check it and compare it with the existant RecentMessageSet and that stuff.
then check the texteditor/paragraphEditor
I'll write it down in my TODO list :). But the idea is to replace ParagraphEditor with them, is it?
Well, this mail is just for ask for advice and argue about the
correctness of this change (or not).
Thanks for reading up to here! Guille _______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Hi Guille, CUIS is Juan Vuletich's Smalltalk-80 environment derived from Squeak. See http://www.jvuletich.org/Cuis/Index.html Francisco 2010/8/10 Guillermo Polito <guillermopolito@gmail.com>:
On Tue, Aug 10, 2010 at 6:00 AM, Stéphane Ducasse <stephane.ducasse@inria.fr> wrote:
Benjamin started and rewrote the recentMessages and friends. If people can review his code it would be great.
Ok, I'll watch it a few minutes this evening.
Related with the issue http://code.google.com/p/pharo/issues/detail?id=2757 (Issue 2757: Refactor StringHolder) I was looking at the StringHolder hierarchy, and well, it's not very nice. Â Playing to see how difficult should it be to refactor the hierarchy and decouple a bit those things, I took Workspace off from there, and writing 2 methods it happened to work well (at least with what I tested, hehe).
Sooo, I wondered why a StringHolder, what responsibilities it has, and what ones should it have... Â I found that most of the messages a StringHolder understands are for browsing things and inspect things, such as the Paragraph editor does :). Â And that's why Workspace could do all his stuff when I made him inherit from Object...
I think that we should     - Replace ParagraphEditor by SmalltalkEditor, TextEditor         (did not check but it should be like that in CUIS and Squeak)
What's CUIS? :(
    In Squeak
        TextMorph >> initialize             "TextMorph initialize"             "Initialize the default text editor class to use"             DefaultEditorClass := SmalltalkEditor.
    - then we should probably rewrite some parts and do not inherit from StringHolder     I never understood why MessageSet is a subclass of Browser :)     But I would focus on the
on the ...?
StringHolder class comment says: "I am a kind of Model that includes a piece of text. Â In some cases, the text can be edited, and in some the text is a method."
And I smell that its main responsibility is to hold a Text or a String in its contents variable, and all its subclasses are there because of that... Â And if so, I don't like that inheritance approach :).
This is exact.
I want to destroy (literally) that hierarchy :). Â And maybe it helps us in coming refactors and cleanings, so we can do them with less fear :P.
So could you     review benjamin code?
This evening I'll check it and compare it with the existant RecentMessageSet and that stuff.
    then check the texteditor/paragraphEditor
I'll write it down in my TODO list :). But the idea is to replace ParagraphEditor with them, is it?
Well, this mail is just for ask for advice and argue about the correctness of this change (or not).
Thanks for reading up to here! Guille _______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
What's CUIS? :(
another squeak fork
- then we should probably rewrite some parts and do not inherit from StringHolder I never understood why MessageSet is a subclass of Browser :) But I would focus on the
on the ...?
paragraphEditor kicking out
StringHolder class comment says: "I am a kind of Model that includes a piece of text. In some cases, the text can be edited, and in some the text is a method."
And I smell that its main responsibility is to hold a Text or a String in its contents variable, and all its subclasses are there because of that... And if so, I don't like that inheritance approach :).
This is exact.
I want to destroy (literally) that hierarchy :). And maybe it helps us in coming refactors and cleanings, so we can do them with less fear :P.
So could you review benjamin code?
This evening I'll check it and compare it with the existant RecentMessageSet and that stuff.
then check the texteditor/paragraphEditor
I'll write it down in my TODO list :). But the idea is to replace ParagraphEditor with them, is it?
yes
Well, this mail is just for ask for advice and argue about the correctness of this change (or not).
Thanks for reading up to here! Guille _______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
Ok, I checked CUIS and it is using SmalltalkEditor and TextEditor as expected, and there is no notice of ParagraphEditor :). In squeak 4.1, there are yet some usages of ParagraphEditor, but in some View objects I don't see in Pharo nor in CUIS. I opened an issue for replacing ParagraphEditor: http://code.google.com/p/pharo/issues/detail?id=2778 Cheers! On Tue, Aug 10, 2010 at 10:12 AM, Stéphane Ducasse < stephane.ducasse@inria.fr> wrote:
What's CUIS? :(
another squeak fork
- then we should probably rewrite some parts and do not inherit
from StringHolder
I never understood why MessageSet is a subclass of Browser :) But I would focus on the
on the ...?
paragraphEditor kicking out
StringHolder class comment says: "I am a kind of Model that includes a
piece of text. In some cases, the text can be edited, and in some the text is a method."
And I smell that its main responsibility is to hold a Text or a String
in its contents variable, and all its subclasses are there because of that... And if so, I don't like that inheritance approach :).
This is exact.
I want to destroy (literally) that hierarchy :). And maybe it helps us in coming refactors and cleanings, so we can do them with less fear :P.
So could you review benjamin code?
This evening I'll check it and compare it with the existant RecentMessageSet and that stuff.
then check the texteditor/paragraphEditor
I'll write it down in my TODO list :). But the idea is to replace ParagraphEditor with them, is it?
yes
Well, this mail is just for ask for advice and argue about the
correctness of this change (or not).
Thanks for reading up to here! Guille _______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
great! Stef On Aug 10, 2010, at 8:19 PM, Guillermo Polito wrote:
Ok, I checked CUIS and it is using SmalltalkEditor and TextEditor as expected, and there is no notice of ParagraphEditor :). In squeak 4.1, there are yet some usages of ParagraphEditor, but in some View objects I don't see in Pharo nor in CUIS. I opened an issue for replacing ParagraphEditor:
http://code.google.com/p/pharo/issues/detail?id=2778
Cheers!
On Tue, Aug 10, 2010 at 10:12 AM, Stéphane Ducasse <stephane.ducasse@inria.fr> wrote:
What's CUIS? :(
another squeak fork
- then we should probably rewrite some parts and do not inherit from StringHolder I never understood why MessageSet is a subclass of Browser :) But I would focus on the
on the ...?
paragraphEditor kicking out
StringHolder class comment says: "I am a kind of Model that includes a piece of text. In some cases, the text can be edited, and in some the text is a method."
And I smell that its main responsibility is to hold a Text or a String in its contents variable, and all its subclasses are there because of that... And if so, I don't like that inheritance approach :).
This is exact.
I want to destroy (literally) that hierarchy :). And maybe it helps us in coming refactors and cleanings, so we can do them with less fear :P.
So could you review benjamin code?
This evening I'll check it and compare it with the existant RecentMessageSet and that stuff.
then check the texteditor/paragraphEditor
I'll write it down in my TODO list :). But the idea is to replace ParagraphEditor with them, is it?
yes
Well, this mail is just for ask for advice and argue about the correctness of this change (or not).
Thanks for reading up to here! Guille _______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
participants (3)
-
Francisco Ortiz Peñaloza -
Guillermo Polito -
Stéphane Ducasse