-
Notifications
You must be signed in to change notification settings - Fork 5
"Matthew backend" to "main" pull request for Jon Perry #29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
… Collaborator and Finance pages
…cluding author tags
…ded JSON + Firestore support
…or to an event. Additional commits include collaboration page, and modifications to other classes
Includes updates to the Event class regarding storage of DateTime and TimeOfDay objects for use by the front end. Updated EventCreator to store the additional classes needed by events_home_page
…ed a User object and Event object as its arguments. Additionally, simplified a lot of the overhead, moved around some code, and made everything cloud storage based.
Took the code that Sharun sent me, modified it, and made it compatible with performing CRUD operations with the cloud.
|
Overall, this was really well done code! There are a lot of things I'm leaving out about singletons, supers, named constructors, caching, and memory consumption, but I am evaluating this at an entry level bar. The biggest thing I see missing, and I know this was done in the nick of time, but I would suggest unit testing and functional testing for every piece of code. Coverage of tests matters greatly. The main point of this is ensuring as an application grows and evolves, we will always catch issues if someone modifies another related piece of work that happens to break functionality of your code. Also helps reviewers have I slapped together this tutorial on class initialization with some getters and their idea of how to init classes via constructor. Again, the most proper way would to have a base/abstract, utilizing getters and setters, error checking and assertions within construction. I didn't follow documentation/commenting standards, just a class object standard: |
| import 'package:flutter/widgets.dart'; | ||
|
|
||
| /// @author [MatthewSheldon] | ||
| /// The [InviteUser] class handles the cloud computing side of inviting a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a solid start to a class comment block!
There is a clear and concise meaning to this class.
After clearly explaining what it is, there should always be a section for initialization, if you have an initialization of said object ( what it takes to build an instance of), and you should always have a usage/code block example. You can find a detailed example in the docs here. This code explanation will make your class objects or any large/key function effective for fresh eyes. These are called backtick fences.
| import 'package:firebase_core/firebase_core.dart'; | ||
| import 'package:flutter/widgets.dart'; | ||
|
|
||
| /// @author [MatthewSheldon] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Author information and high level file docs should be written at the top of file. Typically there is another file for contributors to input information. Here is an example of it in the dart code repository.
| } | ||
|
|
||
| /// Returns the title given to the current expense | ||
| String getExpenseName() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love the usage of getters and setters along with the "pseudo" private variables. Typically you can collapse those getters and setters to being represented like getting the name of a class with this code stub:
String get getName => name;
From darts docs:
class Rectangle {
double left, top, width, height;
Rectangle(this.left, this.top, this.width, this.height);
// Define two calculated properties: right and bottom.
double get right => left + width;
set right(double value) => left = value - width;
double get bottom => top + height;
set bottom(double value) => top = value - height;
}
Thank you again for agreeing to review my code. Most of my commits can be found in the following folders:
Various other integration snippets can be found in the following files:
Feel free to go as hard as you would in a professional setting. Only room to improve 👍 Thanks again 🙏