sgreenholtz / Event-Spider

A web scraping application for events
MIT License
0 stars 0 forks source link

Design/Code Review 1 #3

Open cordylus opened 7 years ago

cordylus commented 7 years ago

Project: Event Spider

Developer: Sebastian Greenholtz

Reviewer: Brian Manning

Category Criteria Rating/Comments
Problem Statement Does it exist? Yes
Does the problem statement accurately describe project purpose? Yes, good elevator pitch. Appealing for an end-user.
Is the problem statement professional? Think of prospective employers viewing this as part of the developer's portfolio. Yes. Consider some cleanup as features are deployed and problems resolved.
What is good? Full project wiki with extensive details.
What could be improved? 2016 goals seem really ambitious. Maybe reflect priority of new features in section, or add a little more detail to the top 2 most important planned features.
Design Documentation Does it exist? Yes
Is the navigation/flow through the application logical and easy to use? Yes
Is the order in which the fields are displayed and form fields entered logical and easy to use? Yes
What data is missing? Would like to see mockup of individual event details view.
Is there data that is not used? Doesn't appear to be.
What is good? Screens have all the elements I'd expect as a new user. The important details are all visible at a glance.
What could be improved? A few rough spots with the flow. How do you access event sources? Is it via the add event button in profile? Would like to see a mockup of the home screen for an authenticated user.
Data model Does it exist? Yes
Is everything on the screens represented in the model? Mostly, not seeing a profile pic in schema. Probably still need to define data model for categories/labels/tags.
Does the model represent good database design? Yes
What is good? Good logical design. Easy to understand.
What could be better? Could be fleshed out more to represent the new features. Maybe a comment or namechange to pass column in user to make it clear that they won't be plain text.
Additional design documents Yes, awesome wiki with links to external libraries.
Application structure in IntelliJ Does it exist? Yes
Is the structure correct for a Maven project? Yes
Are packages and classs appropriately named? Yes
Other comments/notes? Will model my own project structure on this.
JSPs Do they exist? Yes
Is templating used (for example, header.jsp, footer.jsp, etc.)? Yes
Is there business logic mixed in the JSPs? No
CSS? Yes, looks great already.
Other comments/notes? Is style responsive? Maybe add mockups of screens as rendered for mobile browser.
Entities/DAOs/Controllers Do they exist? Yes
Java code quality Are methods single-purpose? Yes
Are classes appropriately-sized classes (no monster classes)? Yes
Are the same lines of code repeated at all? No
Do any classes perform very similar functions that could be candidates for super/subclass relationships? Not that I see.
Are any values hard-coded that should be in a properties file? No
Are variable names descriptive? Yes
Are there many branches or loops that could be simplified or broken up into smaller methods? No
Do the DAOs use Hibernate? No hard-coded sql! Yes
Other comments/notes?
Logging Has log4J been added? Yes
Are there logging statements in the code? Yes
Are appropriate logging levels used? Info, debug, error, for example. Yes
Are there any System.out.printlns in the code? No
Other comments/notes?
Unit Tests Do they exist? Yes
Do the tests pass? I must admint I have not tried them yet :)
What is the current code coverage? ?
Is each test truly a unit test or are they functional tests? Existing test are unit test.
Other comments/notes?
Web Service/API integration Has a web service/api been selected? Yes
What web services/apis might work well with this application?
Independent research topic Has a topic been selected? Yes
What topic/s might fit well in this application?
purpleabab commented 7 years ago

Since my teammate Brian already reviewed Sebastian's code according to the formal metric, I'll just comment on broader topics this time, but remain available to do more structured code review in the future.

  1. Good project, ambitious, already have a large code base (took a while to clone the repo).
  2. Would be nice to see an ERD/ visual of the database design. It would be nice to have a 'design documents' folder.
  3. According to our discussion, and our in-person review of the project, you want the concept of 'user-saved-events,' but I don't think this functionality is well thought out yet. I'd like to see a database design to know how you will save the data indicating what events the user attended or intends to attend. I think some concept of 'state' or 'time' will be necessary -- are you recording what the user DID attend or what they WANT to attend? How will you store key words? If you want to use key words as a helper in knowing which future events to recommend to the user, how will you weigh or match them? I think there's a lot of complexity in doing 'event-recommendation' functionality. It seems pretty ambitious. How will you 'calculate' or otherwise identify what a user 'likes'?