carobro commented 5 years ago

Hallo A²L²H,

könntet ihr vielleicht eine Installationsanleitung oder Ähnliches zu eurem Code schreiben? Ich weiß nicht ganz wo ist welche Dateien finde, welche Art von Tests ihr nutzt und welche Requirements ich so brauche :wink: Danke

Die Gruppe :one:

@carobro @JaFro96 @corneliazy @yhallowiegeht @hilal98

JaFro96 commented 5 years ago

Code Review Report AALLH

@HenFo @Anika2 @cherry13579 @atlanta11950 @lukasjah

Installation and Running of the software

Everything worked fine until:

python masterExtract.py Traceback (most recent call last): File "masterExtract.py", line 3, in <module> import extractGeoDataFromFolder as fext File "/home/jannis/Dokumente/geosoft/Geosoftware-II-AALLH/CLI Tools/extractGeoDataFromFolder.py", line 58 SyntaxError: Non-ASCII character '\xc3' in file /home/jannis/Dokumente/geosoft/Geosoftware-II-AALLH/CLI Tools/extractGeoDataFromFolder.py on line 58, but no encoding declared; see http://python.org/dev/peps/pep-0263/ for details

After fixing that with editing the average-symbol in line 58 of extractGeoDataFromFolder.py, I got:

ImportError: No module named zope.interface

After pip install zope.interface --user the software was running!

Import code into development environment

worked without problems

Architecture

I understand:

the folder "GeoDataExtent_pkg" is the python project ready to publish
the folder "CLI Tools" was your working environment

I did not understand:

where do you use the folder "Transaktionen"?
where do you use the folder "einzelne Tools"?

Checklist

Language Independent

Clean Code

[x] use intention revealing names

getsimilarrecords(self): k?, i? masterExtract.py: res? extractGeoDataFromFolder: env? getBoundingBox.py: line 108: ds? 133: gt?
[x] functions should be small

These are the functions with more than 100 lines of code getsimilarrecords(self) 180loc getTimeExtent(name, path) 200loc getPolygon(name, path) 224loc getBoundingBox(name, path) 314loc
[x] do one thing per function

every case (file format) is in one function(getBoundingBox, getPolygon, getTimeExtent), why not exclude them?
[x] make sure code formatting is applied
[x] use exception rather than return codes
[x] no uncalled or unneeded procedures or any unreachable code

folder: einzelne Tools

Security

[x] avoid duplication of code
[x] Avoid excessive logs for unusual behavior

Performance

[x] Avoid creating unnecessary objects
[x] Keep Synchronized Sections Small

General

[x] Don't ignore exceptions
[x] Check parameters for validity

possible checks: path
[x] Comments are comprehensible and add something to the maintainability of the code

very comprehensive commenting in the following files: getBoundingBox.py, getsimilaritybbox(self, raw=False) commenting could be better in the following files: extractGeoDataFromFolder.py, getTimeExtent.py(geojsonCase), spatialSimilarity.py
[x] Are divisors tested for zero or noise

generalSimilarity.py line36: Couldn't len(filename) be zero?

nuest commented 5 years ago

Prima @JaFro96 und Gruppe :one: - für das CLI-tool schon sehr hilfreich. Nur nochmal zu meinem Verständnis: den Service und die dazugehörigen Erweiterungen konntet ihr nicht ans Laufen bekommen?