[Pharo-project] Issue 67 and CrLfFileStream
I've fixed issue 67. SLICE is in PharoInbox. While looking at the classes I found some things. CrLfFileStream is somehow obsolete. Its new class method creates a MultiByteFileStream. The code in both classes is doubled. I didn't check but I would expect it is close to all of the code. This is again a question if we are willing to break back- ward compatibility. I would opt for removing the class completely. If not I would change MultiByteFileStream in a way that it inherits from CrLfFileStream. This way we could at least remove the doubled code/methods. Or does anybody know reasons why the one or other isn't feasible to do? Furthermore I would remove the handling of line endings with symbols. These could just be the strings they contain. String cr, String lf and String crlf all exist. Using this instead of the symbols we could remove half of the class variables and get rid of the LineEndStrings dictionary. What do you think? And could someone please review my changes. I don't have a Mac. Well, the difference is only noticable if you have a system < 10. Norbert
Maybe the email was not clear enough. The fix is ok. My question are targeted towards additional refactorings. Just in case that is the reason that wasn't been harvested :) Norbert On Sat, 2009-06-13 at 11:17 +0200, Norbert Hartl wrote:
I've fixed issue 67. SLICE is in PharoInbox.
While looking at the classes I found some things. CrLfFileStream is somehow obsolete. Its new class method creates a MultiByteFileStream. The code in both classes is doubled. I didn't check but I would expect it is close to all of the code. This is again a question if we are willing to break back- ward compatibility. I would opt for removing the class completely. If not I would change MultiByteFileStream in a way that it inherits from CrLfFileStream. This way we could at least remove the doubled code/methods.
Or does anybody know reasons why the one or other isn't feasible to do?
Furthermore I would remove the handling of line endings with symbols. These could just be the strings they contain. String cr, String lf and String crlf all exist. Using this instead of the symbols we could remove half of the class variables and get rid of the LineEndStrings dictionary.
What do you think? And could someone please review my changes. I don't have a Mac. Well, the difference is only noticable if you have a system < 10.
Norbert
_______________________________________________ Pharo-project mailing list Pharo-project@lists.gforge.inria.fr http://lists.gforge.inria.fr/cgi-bin/mailman/listinfo/pharo-project
On Jun 13, 2009, at 8:16 PM, Norbert Hartl wrote:
Maybe the email was not clear enough. The fix is ok. My question are targeted towards additional refactorings.
Just in case that is the reason that wasn't been harvested :)
;) I have the impression that we give a quite strong reactivity :) Probably too much :) Stef
Norbert
On Sat, 2009-06-13 at 11:17 +0200, Norbert Hartl wrote:
I've fixed issue 67. SLICE is in PharoInbox.
While looking at the classes I found some things. CrLfFileStream is somehow obsolete. Its new class method creates a MultiByteFileStream. The code in both classes is doubled. I didn't check but I would expect it is close to all of the code. This is again a question if we are willing to break back- ward compatibility. I would opt for removing the class completely. If not I would change MultiByteFileStream in a way that it inherits from CrLfFileStream. This way we could at least remove the doubled code/methods.
Or does anybody know reasons why the one or other isn't feasible to do?
Furthermore I would remove the handling of line endings with symbols. These could just be the strings they contain. String cr, String lf and String crlf all exist. Using this instead of the symbols we could remove half of the class variables and get rid of the LineEndStrings dictionary.
What do you think? And could someone please review my changes. I don't have a Mac. Well, the difference is only noticable if you have a system < 10.
Norbert
_______________________________________________ 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
On Sat, 2009-06-13 at 21:06 +0200, Stéphane Ducasse wrote:
On Jun 13, 2009, at 8:16 PM, Norbert Hartl wrote:
Maybe the email was not clear enough. The fix is ok. My question are targeted towards additional refactorings.
Just in case that is the reason that wasn't been harvested :)
;) I have the impression that we give a quite strong reactivity :) Probably too much :)
There is no tooo much. It is very good indeed :) I was just wondering because I fixed this one before the one Marcus harvested. Norbert
Stef
Norbert
On Sat, 2009-06-13 at 11:17 +0200, Norbert Hartl wrote:
I've fixed issue 67. SLICE is in PharoInbox.
While looking at the classes I found some things. CrLfFileStream is somehow obsolete. Its new class method creates a MultiByteFileStream. The code in both classes is doubled. I didn't check but I would expect it is close to all of the code. This is again a question if we are willing to break back- ward compatibility. I would opt for removing the class completely. If not I would change MultiByteFileStream in a way that it inherits from CrLfFileStream. This way we could at least remove the doubled code/methods.
Or does anybody know reasons why the one or other isn't feasible to do?
Furthermore I would remove the handling of line endings with symbols. These could just be the strings they contain. String cr, String lf and String crlf all exist. Using this instead of the symbols we could remove half of the class variables and get rid of the LineEndStrings dictionary.
What do you think? And could someone please review my changes. I don't have a Mac. Well, the difference is only noticable if you have a system < 10.
Norbert
_______________________________________________ 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 (2)
-
Norbert Hartl -
Stéphane Ducasse