libmir / dcv

Computer Vision Library for D Programming Language
http://dcv.dlang.io/
Boost Software License 1.0
91 stars 18 forks source link

simplify API's with isSlice #115

Closed timotheecour closed 7 years ago

timotheecour commented 7 years ago

here's an example:

Figure plot(SliceKind kind, size_t[] packs, Iterator) (Slice!(kind, packs, Iterator) slice, GGPlotD gg, string title = "");

could be simplified (and be more readable) with:

Figure plot(S)(S slice, GGPlotD gg, string title = "") if(isSlice!S);

there are lots of such cases (search for SliceKind)

9il commented 7 years ago

Why this should be simplified?

timotheecour commented 7 years ago

Figure plot(S)(S slice, GGPlotD gg, string title = "") if(isSlice!S)

is arguably more readable than

Figure plot(SliceKind kind, size_t[] packs, Iterator) (Slice!(kind, packs, Iterator) slice, GGPlotD gg, string title = "")

9il commented 7 years ago

The older style should be used. After your change the following code would not work for example:

    import mir.ndslice;
    const s = slice!double(2, 3);

    auto foo(SliceKind kind, size_t[] packs, Iterator) (Slice!(kind, packs, Iterator) slice)
    {
        slice.popFront;
    }

    foo(s);
9il commented 7 years ago

I agree that (S) is simpler. But a DIP that strip const should be implemented first .

timotheecour commented 7 years ago

isSlice has a confusing name, isX should always be bool-return.

how about:

// before:
template isSlice(T) {
    static if (is(T : Slice!(kind, packs, Iterator), SliceKind kind, size_t[] packs, Iterator))
        enum size_t[] isSlice = packs[];
    else
        enum size_t[] isSlice = null;
}

// after: rename isSlice to getSlicePacks (and update code using it) and define:
template isSlice(T) {
    enum isSlice = is(T : Slice!(kind, packs, Iterator), SliceKind kind, size_t[] packs, Iterator));
}

would that be acceptable? at least as PR?

9il commented 7 years ago

isSlice behaves as boolean if it is used as boolean. It just stores additional information that is very useful for generic code