Skip to content
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

feat(verify.py): implemented function verify and its test #172

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

FilippoMarletta
Copy link

impementented function verify to check whether an int value belongs to a list of integers and implemented its test

src/verify.py Outdated
@@ -0,0 +1,6 @@
def verify(l:list[int], v:int ) -> bool:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sarebbe meglio usare dei nomi più descrittivi sia per il nome della funzione (es: is_element_in_list oppure list_contains), sia per i parametri in ingresso(che chiamerei senza abbreviazione list e value), in modo che, semplicemente leggendo la firma della funziona, si intuisca cosa si aspetta in input, cosa ci aspettiamo che faccia e cosa eventualmente ritorna come output.
In questo caso potrebbe sembrare superfluo perché la funzione è banale, tuttavia è una buona abitudine che è meglio acquisire il prima possibile :)

src/is_element_in_list.py Outdated Show resolved Hide resolved
@makapx makapx requested a review from TendTo November 8, 2023 18:02

def test_is_element_in_list() -> None:
assert is_element_in_list([1,2,3], 2) == True
assert is_element_in_list([1,2,3], 4) == False
Copy link

@simone989 simone989 Nov 9, 2023

Choose a reason for hiding this comment

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

Definisci l'array di elementi [1,2,3] in una costante

Copy link
Author

Choose a reason for hiding this comment

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

ho parametrizzato tutto, va bene così?
commit

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.

5 participants