Add version constants

[lmars]

So that users of this library don't have to use "magic" numbers, so cid.Prefix{Version: 1} becomes cid.Prefix{Version: cid.CIDv1}.

[kevina]

[Deleted earlier comment as it was not well thought out]

Note that cid.Prefix{Version: 1} will still work.

[Stebalien]

I'm not sure I really see the benefit here. CID version numbers aren't magic numbers, they're just numbers.

[kevina]

@Stebalien, for the most part, I agree.

[lmars]

I think using a constant like CIDv0 rather than just 0 is an improvement for the following reasons:

• code matches the wording in the spec which refers to the versions as CIDv0 and CIDv1 (the Go representation of those versions as 0 and 1 is an implementation detail)
• the versions can be documented in code
• it is more readable

I don't see any disadvantages as users who wish to continue to use 0 and 1 are free to do so.

[Stebalien]

I don't see any disadvantages

Two ways to do the same thing is a big disadvantage. Users will see CIDv1 in one case and 1 in another and wonder if they mean the same thing, if the 1 is a bug or legacy code, etc. Worse, people will start arguing about which should be used like we're doing here.

the versions can be documented in code

We can document them at the package level whatever we do.

---

Basically, to me, this feels like the classic:

const ONE = 1
const TWO = ONE + ONE
const THREE = TWO + ONE

[lmars]

Users will see CIDv1 in one case and 1 in another and wonder if they mean the same thing ... people will start arguing about which should be used like we're doing here.

Sorry if I am coming across as argumentative, I am trying to propose a switch to always using the constants CIDv0 and CIDv1 for any field which has type Version, and enforcing that by changing all references in this library. Of course, Go doesn't have true sum types which leaves the possibility of doing this "two ways", but idiomatic Go code will just use the constants (just like idiomatic Go code will use os.SEEK_SET rather than 0).

Basically, to me, this feels like the classic: const ONE = 1 const TWO = ONE + ONE const THREE = TWO + ONE

Well no, there you are aliasing the countably infinite set of integers which is already well represented by numerical digits. Here I am modelling the finite set of valid versions {CIDv0,CIDv1} from the spec with a Version type and two "proposed" values cid.CIDv0 and cid.CIDv1.

This all being said, if you think there is no net improvement of doing this, I can just maintain these constants in my own code, I just thought it would be great to have them in the upstream library.

[Stebalien]

os.SEEK_SET is useful because the choice of 0 as its value is, essentially, arbitrary; it's a true magic number like, e.g., Red, Blue, Green; or Disabled, Enabled, Running. But here, we're dealing with version numbers.

Now, I can think of two valid arguments for this feature:

1. Readability when calling functions. That is, MyCidConstructor(xxx, CIDv1). In this case, there's no field name to tell you what the 1 or 0 is. However, we generally don't write functions like this because different CID versions often behave differently (we usually write different functions for different CID versions).
2. Type safety. That is, if I use CIDv2 in my code (in the future if we ever add a CIDv2) because I saw some tutorial telling me to do so and try to compile against an old cid library, it will fail at compile time (the constant won't exist).

Actually, given the use-case you provided, how about introducing two new functions:

func NewPrefixV0(mhType uint64) Prefix {
       return Prefix{
               MhType:   mhType,
               MhLength: mh.DefaultLengths[mhType],
               Version:  0,
               Codec:    DagProtobuf,
       }
}

func NewPrefixV1(codecType uint64, mhType uint64) Prefix {
       return Prefix{
               MhType:   mhType,
               MhLength: mh.DefaultLengths[mhType],
               Version:  1,
               Codec:    codecType,
       }
}

I already have these functions sitting around in a local branch because they're convenient and it's very easy to accidentally construct an illegal CID prefix.

[lmars]

I'd be happy with the NewPrefixVx methods :+1:.

[lmars]

The need for these constants has been lessened by the changes in #36. Closing.