opencv test draft for ImageContainer #303
Conversation
There was a problem hiding this comment.
Thanks for the work!
I have added some high level comments.
Additionally,
- Can you please format everything with clang-format.
- It is useful to have a descriptive summary of what the function intends to do on top of its definition/declaration.
- It is a good idea to separate logically different lines of code with a newline for better readability.
| return false; | ||
| } | ||
| template<typename T,size_t N> | ||
| bool testcvdp(cv::Mat testImgcv,Img<T,N> testImg,bool norm = false) |
There was a problem hiding this comment.
Can you use a more descriptive name here?
| { | ||
| if(!norm) | ||
| { | ||
| if(!compare_flt(data[k],(T)testImgcv.at<uchar>(i,j))) return false; |
There was a problem hiding this comment.
No need to explicitly return false here, just return the function result directly.
| } | ||
| else | ||
| { | ||
| if(!compare_flt(data[k],(T)testImgcv.at<T>(i,j)))return false; |
There was a problem hiding this comment.
Both of these branches (if and else block) can be simplified using a ternary operator.
| T *data= testImg.getData(); | ||
| if(N==2) | ||
| { | ||
| int k=0; |
There was a problem hiding this comment.
Do you really require signedness from the variable here? You can use unsigned in all cases where it is not required.
| { | ||
| if(a==b) return true; | ||
| float eps = a-b; | ||
| if(eps<0)eps=-eps; |
There was a problem hiding this comment.
You can use std::abs to eliminate an unnecessary branch here.
| if(a==b) return true; | ||
| float eps = a-b; | ||
| if(eps<0)eps=-eps; | ||
| if(eps<FLT_EPSILON) return true; |
There was a problem hiding this comment.
No need for explicit true and false here, you can just return the comparison result itself.
Draft PR for opencv test integration #115