vaimee / zion

A scalable Thing Description Directory
Apache License 2.0
17 stars 4 forks source link

Use `assert` or `require` for input validation methods/functions #19

Closed relu91 closed 2 years ago

relu91 commented 2 years ago

I've also noticed that we have requireValidThingDescription method in the same class. We should decide whether to use assert or require in such cases and write the final decision here (I'm opening an issue to tracking this down)

We have functions that validate input and show if the input is not valid. How we should prefix those functions? In #18 we are suggesting using assert but previously we used require. Which one is more evocative? You can even propose your alternative prefix here. When the discussion is settled we MUST update the CONTRIBUTING.md

relu91 commented 2 years ago

My vote goes to assert. The problem with require is that NodeJS has a well-known function called require 😺

ivanzy commented 2 years ago

I quickly looked at other discussions about the same topic and did not find much. here states that the function stops the execution as the assert function does not. However, I do not strongly feel about either naming convention, and I think both clearly state what the function does. So, I would make a quick decision about it (driven more by gut feeling since there is no correct answer here) and move on.

relu91 commented 2 years ago

here states that the function stops the execution as the assert function does not.

I don't think the discussion there is very relevant. He is talking about the inner workings of a testing framework... is not like a well-known behavior for the two functions.

Nonetheless, I agree with this:

think both clearly state what the function does. So, I would make a quick decision about it (driven more by gut feeling since there is no correct answer here) and move on.

hyperloris commented 2 years ago

I would say go with assert. I agree that using required as a prefix is a bit "strange" in this context. I've been writing too much code in Solidity lately and got carried away :smile:

Following Ivan's advice, I would not be wasting too much time on this choice. If you guys agree I will proceed with a small refactor immediately after #18.