Skip to content

Code Review Thomas#9

Open
thoma4kw wants to merge 1 commit intomasterfrom
thomas_CodeReview2
Open

Code Review Thomas#9
thoma4kw wants to merge 1 commit intomasterfrom
thomas_CodeReview2

Conversation

@thoma4kw
Copy link
Copy Markdown
Collaborator

@thoma4kw thoma4kw commented Apr 3, 2023

Description: The project allows users to customize their room layout with various furniture pieces as well as custom drawing capabilities to help with decoration. Creators also created drag and drop function to test furniture pieces in a given room.

The program is constructed well and commented a throughout to understand

The program was available to be on time

The program runs

Three Concepts I learned:
Functions should not always be public.

New button implementation that cleans up the the format it is coded in.

How to use onClickListener to get pointer position , draw on the screen as done in this app.

References:
https://cswsolutions.com/blog/posts/2022/december/5-benefits-of-unit-testing-and-why-you-should-care/#:~:text=Unit%20tests%20can%20help%20you,released%20into%20the%20production%20environment.
https://kotlinlang.org/docs/reference/coding-conventions.html

@thoma4kw
Copy link
Copy Markdown
Collaborator Author

thoma4kw commented Apr 3, 2023

ea08d8a

Comment thread app/build.gradle
Comment on lines -40 to +47
implementation 'androidx.core:core-ktx:1.7.0'
implementation 'androidx.core:core-ktx:1.9.0'
implementation 'androidx.appcompat:appcompat:1.6.1'
implementation 'com.google.android.material:material:1.8.0'
implementation 'androidx.constraintlayout:constraintlayout:2.1.4'
implementation 'androidx.core:core-ktx:+'
implementation 'androidx.annotation:annotation:1.3.0'
implementation 'androidx.lifecycle:lifecycle-livedata-ktx:2.4.1'
implementation 'androidx.lifecycle:lifecycle-viewmodel-ktx:2.4.1'
implementation 'androidx.core:core-ktx:1.9.0'
implementation 'androidx.annotation:annotation:1.6.0'
implementation 'androidx.lifecycle:lifecycle-livedata-ktx:2.5.1'
implementation 'androidx.lifecycle:lifecycle-viewmodel-ktx:2.5.1'
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Careful when changing dependencies. They tend to have dependencies on each other, so updating one can become a game of whac-a-mole.

Comment on lines -25 to +34
init()
screenSmooth()
}
constructor(context: Context, attrs: AttributeSet?) : this(context, attrs, 0){
init()
screenSmooth()
}

constructor(context: Context, attrs: AttributeSet?, defStyleAttr: Int) : super(context, attrs, defStyleAttr) {
init()
screenSmooth()
}
private fun init(){
private fun screenSmooth(){
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

init is the proper name for an initialization function, as this one appears to be. I'd keep this as it were.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a direct copy of another student's code review.

Comment on lines +49 to +52
fun isValidEmail(email: String): Boolean {
val emailRegex = Regex("^\\w+([.-]?\\w+)*@\\w+([.-]?\\w+)*(\\.\\w{2,})+\$")
return email.matches(emailRegex)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a direct copy of another student's code review.

@discospiff
Copy link
Copy Markdown

Don't merge this branch.

Comment on lines -14 to +29
fun addition_isCorrect() {
assertEquals(4, 2 + 2)
fun `Given a exercise name contained within the json then I should recieve a list of muscles that are worked out by thte exercise`() {
givenJsonFileIsValid()
whenExerciseDataIsReadAndParsed()
thenExerciseCollectionShouldNotBeEmpty()
} private fun thenExerciseCollectionShouldNotBeEmpty() {
TODO("Not yet implemented")
}

private fun whenExerciseDataIsReadAndParsed() {
TODO("Not yet implemented")
}

private fun givenJsonFileIsValid() {
TODO("Not yet implemented")
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants