Url shortener & tests

[ZacharyBohn]

Description

• Adds functions to generate a short url given a real url.
• Adds basics structure for the project
• Adds the foundation for running tests

Todo

• [x] Ensure that when a short url is generated, it is done so deterministically

[coachadelson]

Hi Zachary,

Great job on making significant progress with the URL shortener project! Here are some detailed comments and suggestions to help improve your code further:

General Comments:

• Structure and Organization: The project is well structured so far, with clear separation between service logic and tests. This makes the code easy to read and maintain. Keep this in mind as the project continues to grow.
• Docstrings and Comments: Good use of docstrings in the generate_short_url function. They help in understanding the purpose and functionality of the code. Keep using these.

Specific Feedback:

service/url_shortener.py

1. Functionality:

   • The implementation of URL validation, hashing, and short URL generation is clean and logical so far.
   • Ensure the hashing is deterministic, which you have addressed with your TODO. This is critical for consistent short URL generation.

2. Error Handling:

   • The InvalidUrlException is appropriately raised for invalid URLs. Consider providing a message within the exception to make debugging easier. raise InvalidUrlException("The provided URL is not valid.")

3. Validation Regex:

   • The regex for URL validation is good. However, ensure it covers all edge cases. Testing with a variety of URLs can help ensure it is robust.

4. Hash Function:

   • The hash_url function is straightforward. Consider adding a check to ensure output_length is within reasonable bounds to avoid unexpected behavior.

tests/test_url_shortener.py

1. Test Coverage:

• Your tests cover the primary use cases. Ensure you also test edge cases, such as URLs with query parameters, different protocols, etc as you continue.

• Consider adding tests for different lengths of the shortened URL to verify the flexibility of your hashing function.

  2. Assertions:
  • Good use of assertions. Ensure the error messages are informative if a test fails. Using assertEqual instead of assertEquals (which is deprecated) is recommended. self.assertEqual(short_url1, short_url2)

3. Return Statements in Tests:
   • The return statements at the end of your test functions are unnecessary and can be omitted.

tests.py

• Test Runner:
  • The test runner script is simple and effective. Ensure it catches and reports all test cases.

main.py

• Entry Point:
  • The placeholder for the entry point is good. As the project evolves, you will need to flesh this out to initialize and run your FastAPI application.

Suggestions for Improvement:

1. Expand Test Cases:
   • Add more test cases to cover edge scenarios and different URL formats. This will help ensure the robustness of your URL shortener.
2. Error Handling in Depth:
   • Enhance error messages for better debugging. Consider additional error handling, especially in scenarios where external dependencies might fail.
3. Documentation:
   • Expand the documentation to include usage examples and a brief explanation of each function's purpose in the future. This will make the project easier to understand and use.
4. CI/CD Integration:
   • We will be integrating a CI/CD pipeline (e.g., GitHub Actions) to automate testing and ensure code quality with each commit.

Next Steps:

1. Address the TODO:
   • Ensure deterministic short URL generation as planned.
2. Enhance Error Messages:
   • Provide informative error messages for better debugging.
3. Expand Testing:
   • Add more comprehensive test cases to cover various scenarios and edge cases.
4. Prepare for Deployment:
   • Start thinking about how you will deploy this application (e.g., Dockerize the application, set up AWS resources).

Great progress so far, Zachary! Keep up the good work and let me know if you have any questions or need further assistance.

[ZacharyBohn]

Return Statements in Tests:

The return statements at the end of your test functions are unnecessary and can be omitted.

I find that since Python does not use curly braces to delimit function scopes, that a dangling return statement makes it a bit more readable. Though I do understand that this is not a common practice.