Starting from the top:\
(0) It's not *wrong* to start a selector with a capital letter, but
it is certainly unusual.
(1) Intention-revealing names are good. That's why this name is bad.
It does not *delete* anything.
(2) The comment again says "delete" but nothing is deleted. Comments
that mislead are worse than no comments.
(3) An issue I have seen over and over in student code (in other
programming languages) is writing regular expressions as string
literals where they are used, so that every time you try a match
the regular expression gets recompiled, over and over again.
You're not exactly doing that, but you *are* rebuilding patterns
over and over again. It would make more sense to hold a
forbiddenPatterns collection.
(4) You are saying "(coll reject: [..test..]) notEmpty", building an
entire collection just to ask whether it has any members. What
you want to know is "are there any elements of coll that do not
satisfy the test", which can be expressed as
(coll allSatisfy: [..test..]) not. (Sadly there is no
#notAllSatisfy:, although #notAnySatisfy: *does* exist under the
name #noneSatisfy:.
(5) #match: is a highly risky way to look for a substring. What if
you want to look for a literal # or * ? What if you need to
look for 'Boring' but not 'boring'? ("Boring" is a perfectly
good surname.)
(6) You really don't need the "result" variable.
allowedWords
"Answer those words which do not contain any forbiddenWord
as a substring."
^words select: [:each |
forbiddenWords noneSatisfy: [:forbidden |
<<each includes forbidden as a substring>>]]
(7) You can find out how to do the <<part>> by looking at the methods
for String. You will find several candidate methods.
(8) Of course, if there are W words and F forbidden words, in the
worst case you'll do W*F substring inclusion tests. This is not
good. "Multiple pattern matching" is a much-studied problem,
see for example Wu and Manber's paper
http://web.archive.org/web/20130203031412/http://www.inf.fu-berlin.de/inst/ag-bio-expired/FILES/ROOT/Teaching/Lectures/WS0506/WeiterfThemenBioinf/papers/FastAlgorithmMultiPatternSearching-WuManbers-TR94.pdf
There is more recent work also.
Does anyone know whether there's a multiple pattern algorithm for
Pharo? One could always fall back on ternary search trees...
On Fri, 14 Dec 2018 at 05:13, Roelof Wobben <r.wob...@home.nl> wrote:
> Hello,
>
> I have this code :
>
> FindAndDeleteWordsWithForbiddenParts
> "deletes all the words that contain forbidden parts"
>
> | result |
> result := words
> select: [ :word |
> [ (forbiddenWords reject: [ :forbidden | '*' , forbidden , '*'
> match: word ]) isNotEmpty ]
> on: NotFound
> do: [ false ] ].
> ^ result
>
>
> but I see warnings that I have to use a stream instead of string concentation.
>
> Anyone hints how to do so ?
>
> Roelof
>
>
>
