crytic / crytic/properties

[Bug-Candidate]: Rename "clamp" to avoid confusion

Open
#49 0 comments 1 reaction 0 assignees View on GitHub
Dominant language
Solidity
Stars
371
Forks
59
PR merge metrics
No merged PRs in 30d

Description

### Describe the issue:

### The state

There's a set of functions in `contracts/util/PropertiesHelper.sol` named `clamp...` for making numbers fit in a fixed range by performing modulo. For example using such function to put numbers 0-10 in the range 4-6 will result in:

```
in: 0 1 2 3 4 5 6 7 8 9 10
out: 6 4 5 6 4 5 6 4 5 6 4
```

### The problem

In the programming nomenclature the term "clamp" usually refers to putting number in a fixed range by assigning it the value of the boundary, if that boundary is exceeded. For example clamping numbers 0-10 in the range 4-6 would result in:

```
in: 0 1 2 3 4 5 6 7 8 9 10
out: 4 4 4 4 4 5 6 6 6 6 6
```

I'm writing this based on my experience, I got really confused when I saw `PropertyHelper`'s `clamp...` functions in use for the first time. I checked my expectations by searching on the internet for "clamp a number", and all the resources were referring to "clamping" as assigning the exceeded boundary, not calculating the modulo. I think that many developers using `PropertiesHelper` for the first time will be confused or misled.

### The solution

I think that the `clamp...` functions should be renamed to something less confusing. For example the name `wrap...` could be good, it would reflect how numbers exceeding the limit are returning into the range on the other end.

### Steps to reproduce the issue:

Write code using `PropertiesHelper`.

### If additional code is needed for reproducing, please copy it here, or drop us a link to the repository:

_No response_

### Echidna version:

All of them.

### Additional information:

_No response_

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.