Skip to content

Conversation

@filipmarkovic064
Copy link

@filipmarkovic064 filipmarkovic064 commented Jan 6, 2026

Hello and thank you for the review! I have a few questions if that is alright :)

  1. Is my PhonebookController overloaded? I feel like theres too much stuff there but its just basic CRUD stuff so moving it elsewhere seems kind of wrong?
  2. I tried following the tip you gave me after the DrinksInfo Project :

"This is more related to architecture but you're mixing HTTP stuff with UI stuff. You want to separate them so each method is easier to test."

Is this better? i tried not mixing business logic with UI stuff, however that made it so i have to write a bunch of extra stuff to make sure the app doesn't crash, which makes it look a bit more stuffed, is this what you meant tho and is this alright?

  1. I tested my Validation of user inputs, is there anything else i should've tested with unit tests?
  2. I also had to switch to public instead of internal in order to test my UserInputController, is this unsafe?

Btw i didnt want to deal with subscriptions on Twilio so i didnt implement the SMS functionality but it seemed simple enough using their API according to StackOverflow

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.

1 participant