Re: [Aramorph-users] A contribution for AraMorph

[Pierrick Brihaye]

Hi again,

Pierrick Brihaye a écrit :

> >   This is a piece of work I have made in the AraMorph project.
>  
> Nothing is actually attached AFAICS :-(

Sorry. Since your message was > 40 kb, I've had to moderate it... and 
the delay is quite long.

Well... let's discuss :-)

First, you didn't work on the very latest version. No problem though...

>> 3- Used log4j package for outputting messages instead of System.out
>> AraMorph.java

Good idea. We will have to provide a command line parameter for the log 
configuration file and/or for the log file.

However, lenghty processes (the ........), may perhaps give feedback on 
the console.

>> InMemoryDictionaryHandler.java
>> 1- Move the Pattern p declaration to the class level. This makes it
>> created once.

You're right.

You also added qualified packages :
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Set;

So, I wonder whether this line is still useful :
import java.util.*;

Put your name in the javadoc (@author), not here :
//<By Ahmed Saad>

Don't remove these comments :

// two ways to get the POS info:
// (1) explicitly, by extracting it from the gloss field:

Because you have to add this one :
//TODO Ahmed Saad: try to understand this block

I know, this piece of the code is difficult and you may have to trace 
it. It is due to the fact that Buckwalter's dictionaries sometimes 
include POS information in the GLOSS. I would have prefered a better POS 
segmentation rather than this tricky workaround. Anyway...

You shouldn't comment this line :
/*gloss = gloss.replaceAll(";","/"); //TODO : is it necessary ?

... because it may have side effects :-)

Typo :

* will be traversed one by one and replacted according

Shouldn't this be static :
private String translate(String gloss) {

Otherwise, it looks OK. One more thing however :

We should stick to ASCII source code. Thus we should escape arabic (and 
french) characters using the Java \uXXXX syntax.

Cheers,

p.b.