Per Møldrup-Dalum

To what is this comment pertaining?

To what is this comment pertaining?

great — then maybe change the name to something that makes a bit more sense when looking at the file in the file structure (on the file system or in version control system)

great — then maybe change the name to something that makes a bit more sense when looking at the file in the file structure (on the file system or in version control system)

som rettes?

som rettes?

Jeg spørger om "component" i dette tilfælde klassen StructureValidator? Hvis den ikke er, så referer "component" til noget overordnet til denne klasse og så mener jeg det skal nævnes. Hvis der ER d...

Jeg spørger om "component" i dette tilfælde klassen StructureValidator? Hvis den ikke er, så referer "component" til noget overordnet til denne klasse og så mener jeg det skal nævnes. Hvis der ER denne klasse, hvorfor så ikke bare skrive "Get the name of this class..."?

Så er det måske noget mere overordnet der kunne gøres anderledes. Jeg påpeger kun dette da anvendelse af "instanceof" måske kan være tegn på et problem i designet som på sigt vil gøre vedligehold b...

Så er det måske noget mere overordnet der kunne gøres anderledes. Jeg påpeger kun dette da anvendelse af "instanceof" måske kan være tegn på et problem i designet som på sigt vil gøre vedligehold besværlig.

Jeg spørger for at få dig til at tænke over dette — og for selv at lære noget.

Jo, men derfor skal det vel alligevel være i orden? Jeg synes stadig de skal ordnes.

Jo, men derfor skal det vel alligevel være i orden? Jeg synes stadig de skal ordnes.

OK, så betydningen af CompleteCheckFactory har ikke ændres sig, kun metoden klassen benytter?

OK, så betydningen af CompleteCheckFactory har ikke ændres sig, kun metoden klassen benytter?

before the task is done?

before the task is done?

Commented out test of not-yet-implemented functionality.
Commented out test of not-yet-implemented functionality.
Lad os snakke om mine specifikke kommenterer…

Lad os snakke om mine specifikke kommenterer…

Hov, en mere instanceof. Det er sikkert mig der ikke ved hvordan man programmerer i disse frameworks, men på mig er det en rød lampe der blinker orange...

Hov, en mere instanceof. Det er sikkert mig der ikke ved hvordan man programmerer i disse frameworks, men på mig er det en rød lampe der blinker orange...

"indent" forstår jeg ud (måske) udfra navnet, men hvad er extra? Jeg mener fields i en klasse skal forklares

"indent" forstår jeg ud (måske) udfra navnet, men hvad er extra? Jeg mener fields i en klasse skal forklares

Hvad er en "component" i denne sammenhæng?

Hvad er en "component" i denne sammenhæng?

Hvorfor bruges instanceof i stedet for polymorfi (eller andre metoder)?

Hvorfor bruges instanceof i stedet for polymorfi (eller andre metoder)?

Hvorfor er XmlBuilderEventHandler så forskellige fra de andre eventHandlers?

Hvorfor er XmlBuilderEventHandler så forskellige fra de andre eventHandlers?

Er det nødvendigt at have dette filnavn hårdkodet her? Nu ved jeg (endnu) ikke hvordan man Schematron validerer fra Java, men hvis schematron koden er låst kunne den måske lige så godt være in-line...

Er det nødvendigt at have dette filnavn hårdkodet her? Nu ved jeg (endnu) ikke hvordan man Schematron validerer fra Java, men hvis schematron koden er låst kunne den måske lige så godt være in-lined i Javakoden?

Forklar lige hvorfor denne løsningsmåde er nødvendig

Forklar lige hvorfor denne løsningsmåde er nødvendig

Hvorfor er kode udkommenteret i stedet for slettet?

Hvorfor er kode udkommenteret i stedet for slettet?

Hvilken rolle har denne klasse?

Hvilken rolle har denne klasse?

Jeg mener at import erklæringer skal grupperes MINDST med adskillelse af vores egne namespaces og "de andres". Ikke blandes sammen

Jeg mener at import erklæringer skal grupperes MINDST med adskillelse af vores egne namespaces og "de andres". Ikke blandes sammen

her ville jeg også vælge at have "then" på anden linie if [[ … ]] then … fi da dette er mere idiomatisk Bash og ikke et forsøg på at lægge sin egen stil ned over et sprog.

her ville jeg også vælge at have "then" på anden linie
if [[ … ]]
then

fi
da dette er mere idiomatisk Bash og ikke et forsøg på at lægge sin egen stil ned over et sprog.

Hen i mod slutningen af mit review så jeg at jeg kiggede på lidt ældre filer. Det var "runNo" som satte mig på sporet. Ignorer derfor mine kommentarer om netop "runNo". Jeg har slet ikke kigget på...

Hen i mod slutningen af mit review så jeg at jeg kiggede på lidt ældre filer. Det var "runNo" som satte mig på sporet. Ignorer derfor mine kommentarer om netop "runNo".

Jeg har slet ikke kigget på testkoden.

Der mangler en beskrivelse af hvad rollen af dette kodeprojekt er? Hvilke klasser og interfaces er med i "pakken" og hvad er deres indbyrdes relationer? Jeg kunne godt tænke mig at se et diagram/en...

Der mangler en beskrivelse af hvad rollen af dette kodeprojekt er? Hvilke klasser og interfaces er med i "pakken" og hvad er deres indbyrdes relationer? Jeg kunne godt tænke mig at se et diagram/en tegning over klasserne. Gerne ASCII eller en PNG/JPG fil.

Er der steder i implementation som med fordel kunne få et par ord med på vejen? Noget genial kode? Et sted hvor der i denne version er hoppet over hvor gærdet er lavest? Er der svagheder der skal tages hensyn til senere?

Hvilket generaliseringsniveau er valgt?

Jeg går ud fra at denne licens er tilføjet med Mavens Licens plugin?

Jeg går ud fra at denne licens er tilføjet med Mavens Licens plugin?

Hvilken rolle har denne Factoryklasse?

Hvilken rolle har denne Factoryklasse?

Jeg kan meget bedre lide denne formattering end de ret lange method definitioner.

Jeg kan meget bedre lide denne formattering end de ret lange method definitioner.

Hvad betyder "do not think the class will clone and return".

Hvad betyder "do not think the class will clone and return".

Then log it before review.

Then log it before review.