COIN-OR::LEMON - Graph Library

Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#104 closed defect (fixed)

Meaningless parameter in ArgParser::boolOption()

Reported by: Balazs Dezso Owned by: Alpar Juttner
Priority: minor Milestone: LEMON 1.0 release
Component: core Version: hg main
Keywords: Cc:
Revision id:

Description

Usually, the value parameter is used to add a default value for a argument. However in the boolean case, if this parameter is set true, then the argument will always true, because false argument cannot be given from command line. So, I think this parameter is meaningless in this context.

Attachments (3)

arg_parser_doc_eea4295af4ad.patch (8.7 KB) - added by Peter Kovacs 11 years ago.
arg_parser_doc_1767a4db2b8c.patch (9.0 KB) - added by Peter Kovacs 11 years ago.
Doc improvements
arg_parser_doc_77d56a21c3ab.patch (9.0 KB) - added by Peter Kovacs 11 years ago.
Doc improvements

Download all attachments as: .zip

Change History (14)

comment:1 in reply to:  description Changed 11 years ago by Alpar Juttner

Replying to deba:

So, I think this parameter is meaningless in this context.

Yes it is. What is more, the last parameter (obl) doesn't make sense either. But this way of design was intential, for two reasons.

  • For the sake of the uniform interface
  • In the future we may want to introduce a syntax for setting a bool parameter to false. This design leaves this possibility open.

comment:2 Changed 11 years ago by Alpar Juttner

Status: newassigned
Version: hg main

Changed 11 years ago by Peter Kovacs

comment:3 Changed 11 years ago by Peter Kovacs

I uploaded a patch of doc improvements for ArgParser?.

comment:4 in reply to:  3 ; Changed 11 years ago by Alpar Juttner

Replying to kpeter:

I uploaded a patch of doc improvements for ArgParser?.

The changeset [eea4295af4ad] contains very long lines. We should stick on the 80 character upper limit. (And probably it should also be explicitly written in the coding conventions.)

comment:5 in reply to:  4 ; Changed 11 years ago by Peter Kovacs

Replying to alpar:

We should stick on the 80 character upper limit. (And probably it should also be explicitly written in the coding conventions.)

I am not really sure. In most cases we should keep this limit, but sometimes using long lines seems to be a better choice, since it results more clearly arranged code, especially in test/demo files.

Do you have any idea to make this changeset better without losing the line comments?

comment:6 in reply to:  5 Changed 11 years ago by Alpar Juttner

Replying to kpeter:

Replying to alpar:

I am not really sure. In most cases we should keep this limit, but sometimes using long lines seems to be a better choice, since it results more clearly arranged code, especially in test/demo files.

In no way. Broken lines are much more difficult to read and long line can be avoided.

Vast majority of projects treat it as a strict requirement and we should do so as well.

Do you have any idea to make this changeset better without losing the line comments?

Some ideas:

  • Use less spaces
  • Break the lines
  • Use doxygen comments outside the source code like in case of grapg_orinentation.cc. It results in a documentation which is pleasant to read.

Changed 11 years ago by Peter Kovacs

Doc improvements

comment:7 Changed 11 years ago by Peter Kovacs

[1767a4db2b8c] is almost the same as [eea4295af4ad], but it does not contain long lines.

comment:8 in reply to:  7 ; Changed 11 years ago by Alpar Juttner

Replying to kpeter:

[1767a4db2b8c] is almost the same as [eea4295af4ad], but it does not contain long lines.

Unfortunately, it does not compile.

Changed 11 years ago by Peter Kovacs

Doc improvements

comment:9 in reply to:  8 Changed 11 years ago by Peter Kovacs

Replying to alpar:

Unfortunately, it does not compile.

I missed a semicolon. [77d56a21c3ab] fix it.

comment:10 Changed 11 years ago by Alpar Juttner

Resolution: fixed
Status: assignedclosed

comment:11 in reply to:  description Changed 11 years ago by Alpar Juttner

See ticket:139 for a follow-up.

Note: See TracTickets for help on using tickets.