Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

save infix of const String to infix variable #1009

Closed
kaktus42 opened this issue Jun 3, 2015 · 6 comments
Closed

save infix of const String to infix variable #1009

kaktus42 opened this issue Jun 3, 2015 · 6 comments

Comments

@kaktus42
Copy link

kaktus42 commented Jun 3, 2015

on dev branch.

seqan::Infix<TBarcodeString>::Type referenceIfx =
         infix(representative(indexIt),
               parentRepLength(indexIt) + 1,
               endPos);

throws compiling error:

--------- shortened ---------
/opt/seqan/seqan-src/include/seqan/sequence/segment_infix.h:135:42:
error: invalid conversion from
‘seqan::Pointer_<const TBarcodeString::Type {aka const TBarcodeString*}’ to
‘seqan::Pointer_<TBarcodeString>::Type {aka TBarcodeString*}’
[-fpermissive]
         data_host(_toPointer(host(_other))),

--------- original ---------
/opt/seqan/seqan-src/include/seqan/sequence/segment_infix.h:135:42: error: invalid conversion from ‘seqan::Pointer_<const seqan::String<seqan::SimpleType<unsigned char, seqan::Dna5_>, seqan::Alloc<> > >::Type {aka const seqan::String<seqan::SimpleType<unsigned char, seqan::Dna5_>, seqan::Alloc<> >*}’ to ‘seqan::Pointer_<seqan::String<seqan::SimpleType<unsigned char, seqan::Dna5_>, seqan::Alloc<> > >::Type {aka seqan::String<seqan::SimpleType<unsigned char, seqan::Dna5_>, seqan::Alloc<> >*}’ [-fpermissive]
         data_host(_toPointer(host(_other))),

with

typedef seqan::Dna5String TBarcodeString
typedef seqan::StringSet<TBarcodeString> TBarcodeSet
typedef seqan::Index<TBarcodeSet> TBarcodeIndex
seqan::Iterator<TBarcodeIndex, seqan::TopDown<ParentLinks<> > >::Type indexIt
unsigned endPos

Shouldn't this work properly?

@kaktus42 kaktus42 changed the title save infix of pointer to infix variable save infix of const String to infix variable Jun 3, 2015
@esiragusa
Copy link
Member

Should be:

seqan::Infix<TBarcodeString const>::Type referenceIfx = ...

If you have C++11 you can do:

auto referenceIfx = ...

@kaktus42
Copy link
Author

kaktus42 commented Jun 3, 2015

That indeed works...
Thank you!

A question to this auto feature:
What is better style? Write out the Type or use auto?

@kaktus42 kaktus42 closed this as completed Jun 3, 2015
@esiragusa
Copy link
Member

Hmm hard to say in general. In this case I would prefer auto. Seems to me that auto is always convenient whenever a return type is generic, as you spare computing it through a metafunction.

@h-2
Copy link
Member

h-2 commented Jun 4, 2015

A question to this auto feature:
What is better style? Write out the Type or use auto?

We don't have a policy on this in SeqAn, yet. I recommend to never use auto, unless

  • the inferred type is obvious and
  • it improves readability of the code

So in your example it would be ok, because it is clear that the infix() function returns an infix and the Metafunction just makes it harder to read.

@esiragusa
Copy link
Member

Hmm what's your definition of obvious?

Wouldn't you do:

auto repr = representative(indexIt)

This also returns an infix but it's not so obvious (infix of the const text on which the iterator's index is based on). Indeed that's why @kaktus42 got a problem.

@h-2
Copy link
Member

h-2 commented Jun 4, 2015

Hmm what's your definition of obvious?

There is a trade-off between verbosity and readability, and there are benefits to both, it's definitely something we should discuss. It also depends on the documentation of said function...

For function return values this will also be relevant. And since cxx14, there is also decltype(auto).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants