perander/cfg-parsing

cfg-parsing's Issues

Vertaisarviointi

Projekti ladattu 27.11 klo 17.25

En saanut ohjelmaa toimimaan ennen kuin lisäsin build.gradleen seuraavat konfiguraatiot:

apply plugin: 'application'

mainClassName = 'Main'

run{
standardInput = System.in
}

Yritin ajaa ohjelmaa myös startFileReaderin kautta, mutta resources/input.txt -tiedostoa ei tullut projektin mukana. Yritin lisätä tiedoston, mutta ei ollut oikein selvää, missä muodossa sen sisällön tulisi olla. Olisi käyttäjäystävällistä laittaa githubiin jokin tiedosto valmiiksi. Dokumentoitu esimerkkisyöte tekstikäyttöliittymälle voisi myös olla paikallaan. Pieni käytettävyysparannus olisi laittaa "give a sentence"-osio looppiin, kunnes painetaan esim. enteriä.

Aihe on minulle uusi enkä ehtinyt perehtyä siihen tarkasti, mutta koodi on hyvin ymmärrettävää. Muuttujien ja metodien nimentä on selkeätä ja ohjelman eri komponentit on jaoteltu selkeästi eri pakkauksiin ja luokkiin. JavaDocilla ja kommenteilla on myös hyvin selitetty metodien toimintaa, mutta Rule, State ja Grammar-luokille voisi vielä kirjoittaa JavaDoc-selityksen. Testaus vaikuttaa myös hyvin kattavalta. Kaiken kaikkiaan koodi on niin hyvälaatuista, että en keksi mitään parannettavaa.

En ole aivan varma, ymmärsinkö tarkoituksenmukaista syötettä oikein, mutta algoritmi ei tunnista esim. seuraavaa kieltä, joka on ymmärtääkseni Chomskyn normaalimuodossa:

S -> AB
A -> a
B -> b

Algoritmi vastaa false syötteillä "ab" ja "a b"

Tähän kieleen kuitenkin vastataan true syötteellä "a b":

S -> AA | BB
AA -> a
BB -> b

Toisaalta taas kielellä "S -> a" ja syöteellä "a", CYK vastaa true ja Earley vastaa false.

Siis joko algoritmien toiminnassa on vielä parantamisen varaan, tai sitten vain ymmärsin tarkoitetut syötteet väärin.

Peer review

Project cloned 5.12. 12:00

Building the project using gradle seems to fail because of checkstyle errors. I'm not sure if this is a known issue but maybe should be fixed.
Also while running tests 8 of them seem to fail. Although, based on week report this is know issue.

Because there is no user guide yet, I'm not completely sure if I understood the UI commands correctly. I tried the algorithms with following input:

S -> SS | VP | NP
VP -> a
NP -> b

Now, I think, string 'ab' should belong to the language, but the program gives false as a result.
I tested also example inputs from given by previous reviewer, they seemed to be working fine.

• Maybe it'd be nice to add some examples of context free grammar -inputs and outputs to the documentation so the user could use them as reference.

• Also the UI could accept rules-input in the form S -> SS | VP with characters -> and | instead of spaces, to make it more clear.

• The code was of good quality: Variables were named clearly and javadoc comments were very specific. Although some of the classes and methods were missing comments.

• Test coverage seems to be high with almost every important class and test cases were clear and well written.

All in all the idea and the implementation of the project both seem to be very good.