Skip to content

Commit

Permalink
task: rationalise default value setting in Parameter
Browse files Browse the repository at this point in the history 
The setDefault and getDefault methods are now named setStringDefault
and getStringDefault.  A few type-specific default setter methods
have been added to some of the parameter subclasses.
This is less confusing, more extensible, and easier to use.
  • Loading branch information
@mbtaylor
mbtaylor committed Aug 22, 2014
1 parent d5103f1 commit 4850722
Show file tree
Hide file tree
Showing 73 changed files with 235 additions and 209 deletions.
2 changes: 1 addition & 1 deletion ndtools/src/main/uk/ac/starlink/ndtools/Diff.java
View file
Expand Up @@ -43,7 +43,7 @@ public Diff() {
ndiffpar = new IntegerParameter( "ndiffs" );
ndiffpar.setPrompt( "Number of pixel diffs displayed" );
ndiffpar.setPosition( 3 );
ndiffpar.setDefault( "4" );
ndiffpar.setIntDefault( 4 );
}

public String getPurpose() {
Expand Down
4 changes: 2 additions & 2 deletions task/src/main/uk/ac/starlink/task/BooleanParameter.java
View file
Expand Up @@ -37,8 +37,8 @@ public boolean booleanValue( Environment env ) throws TaskException {
*
* @param dflt default value
*/
public void setDefault( boolean dflt ) {
setDefault( dflt ? "true" : "false" );
public void setBooleanDefault( boolean dflt ) {
setStringDefault( dflt ? "true" : "false" );
}

public Boolean stringToObject( Environment env, String stringval )
Expand Down
4 changes: 2 additions & 2 deletions task/src/main/uk/ac/starlink/task/ChoiceParameter.java
View file
Expand Up @@ -147,15 +147,15 @@ public void setUsage( String usage ) {
public void setDefaultOption( T option ) {
if ( option == null ) {
if ( isNullPermitted() ) {
setDefault( null );
setStringDefault( null );
}
else {
throw new IllegalArgumentException( "null value not allowed" );
}
}
else {
if ( optionMap_.containsKey( option ) ) {
setDefault( getName( option ) );
setStringDefault( getName( option ) );
}
else {
throw new IllegalArgumentException( "No such option: "
Expand Down
9 changes: 9 additions & 0 deletions task/src/main/uk/ac/starlink/task/DoubleParameter.java
View file
Expand Up @@ -34,6 +34,15 @@ public double doubleValue( Environment env ) throws TaskException {
return objVal == null ? Double.NaN : objVal.doubleValue();
}

/**
* Sets the default value as a floating point value.
*
* @param dflt new default value
*/
public void setDoubleDefault( double dflt ) {
setStringDefault( Double.isNaN( dflt ) ? "" : Double.toString( dflt ) );
}

/**
* Sets the minimum acceptable value for this parameter.
*
Expand Down
9 changes: 9 additions & 0 deletions task/src/main/uk/ac/starlink/task/IntegerParameter.java
View file
Expand Up @@ -54,6 +54,15 @@ public int intValue( Environment env ) throws TaskException {
return objectValue( env ).intValue();
}

/**
* Sets the default value as an integer.
*
* @param dflt new default value
*/
public void setIntDefault( int dflt ) {
setStringDefault( Integer.toString( dflt ) );
}

/**
* Mandates that any value of this parameter must be even.
*/
Expand Down
13 changes: 7 additions & 6 deletions task/src/main/uk/ac/starlink/task/LineEnvironment.java
View file
Expand Up @@ -303,15 +303,15 @@ private String promptForValue( Parameter param ) throws TaskException {
/* Assemble a prompt string. */
String name = param.getName();
String prompt = param.getPrompt();
String dflt = param.getDefault();
String strDflt = param.getStringDefault();
StringBuffer obuf = new StringBuffer( param.getName() );
if ( prompt != null ) {
obuf.append( " - " )
.append( prompt );
}
if ( dflt != null || param.isNullPermitted() ) {
if ( strDflt != null || param.isNullPermitted() ) {
obuf.append( " [" )
.append( dflt )
.append( strDflt )
.append( "]" );
}
obuf.append( ": " );
Expand Down Expand Up @@ -343,7 +343,7 @@ private String promptForValue( Parameter param ) throws TaskException {

/* Massage it if necessary. */
String stringVal = sval.length() == 0
? param.getDefault()
? param.getStringDefault()
: readValue( sval );
if ( NULL_STRING.equals( stringVal ) ) {
stringVal = null;
Expand Down Expand Up @@ -422,14 +422,15 @@ public void acquireValue( Parameter param ) throws TaskException {
* the user. */
else if ( interactive_ &&
( promptAll_
|| ( param.getDefault() == null && ! param.isNullPermitted() )
|| ( param.getStringDefault() == null &&
! param.isNullPermitted() )
|| param.getPreferExplicit() ) ) {
stringVal = promptForValue( param );
}

/* Otherwise, use the default. */
else {
stringVal = param.getDefault();
stringVal = param.getStringDefault();
setValueFromString( param, stringVal );
}

Expand Down
2 changes: 1 addition & 1 deletion task/src/main/uk/ac/starlink/task/MultiTaskInvoker.java
View file
Expand Up @@ -196,7 +196,7 @@ private String getUsage( String taskName, Task task ) {
Parameter param = params[ i ];
ubuf.append( padding );
boolean optional = param.isNullPermitted()
|| param.getDefault() != null;
|| param.getStringDefault() != null;
ubuf.append( optional ? "[" : "" )
.append( param.getName() )
.append( '=' )
Expand Down
2 changes: 1 addition & 1 deletion task/src/main/uk/ac/starlink/task/OutputStreamParameter.java
View file
Expand Up @@ -24,7 +24,7 @@ public OutputStreamParameter( String name ) {
super( name, Destination.class, true );
setUsage( "<out-file>" );
setPrompt( "Location of output file" );
setDefault( "-" );
setStringDefault( "-" );

setDescription( new String[] {
"<p>The location of the output file. This is usually a filename",
Expand Down
15 changes: 9 additions & 6 deletions task/src/main/uk/ac/starlink/task/Parameter.java
View file
Expand Up @@ -30,7 +30,7 @@ public abstract class Parameter<T> {
private String prompt_;
private String description_;
private String usage_ = "<value>";
private String dflt_;
private String stringDflt_;
private int pos_;
private boolean nullPermitted_;
private boolean preferExplicit_;
Expand Down Expand Up @@ -418,17 +418,20 @@ public void setPreferExplicit( boolean prefer ) {
*
* @return the default string value
*/
public String getDefault() {
return dflt_;
public String getStringDefault() {
return stringDflt_;
}

/**
* Sets the default string value for this parameter.
* Concrete subclasses may additionally supply type-specific
* default value setter methods, but those ought to operate by invoking
* this method.
*
* @param dflt the default string value
* @param stringDflt the default string value
*/
public void setDefault( String dflt ) {
dflt_ = dflt;
public void setStringDefault( String stringDflt ) {
stringDflt_ = stringDflt;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion task/src/main/uk/ac/starlink/task/SingleTaskInvoker.java
View file
Expand Up @@ -116,7 +116,7 @@ public String getUsage() {
.append( padding );
Parameter param = params[ i ];
boolean optional = param.isNullPermitted()
|| param.getDefault() != null;
|| param.getStringDefault() != null;
ubuf.append( optional ? "[" : "" )
.append( param.getName() )
.append( '=' )
Expand Down
8 changes: 4 additions & 4 deletions task/src/main/uk/ac/starlink/task/TerminalEnvironment.java
View file
Expand Up @@ -116,8 +116,8 @@ private void acquireValue( Parameter par, int ntries )
}

if ( ! valueMap.containsKey( par ) ) {
if ( par.getDefault() != null ) {
valueMap.put( par, par.getDefault() );
if ( par.getStringDefault() != null ) {
valueMap.put( par, par.getStringDefault() );
}
else if ( par.getPreferExplicit() || ! par.isNullPermitted() ) {
String prompt = par.getPrompt();
Expand All @@ -142,8 +142,8 @@ else if ( par.getPreferExplicit() || ! par.isNullPermitted() ) {
value = "";
}
if ( value.length() == 0 ) {
String def = par.getDefault();
if ( par.getDefault() != null ) {
String def = par.getStringDefault();
if ( par.getStringDefault() != null ) {
value = def;
}
else { // recurse
Expand Down
2 changes: 1 addition & 1 deletion task/src/main/uk/ac/starlink/task/TerminalInvoker.java
View file
Expand Up @@ -233,7 +233,7 @@ else if ( pos2 < pos1 ) {
for ( Iterator<Parameter> it = paramList.iterator(); it.hasNext(); ) {
Parameter param = it.next();
boolean optional = param.isNullPermitted()
|| param.getDefault() != null;
|| param.getStringDefault() != null;
if ( optional ) {
usage.append( '[' );
}
Expand Down
11 changes: 7 additions & 4 deletions task/src/testcases/uk/ac/starlink/task/ArithmeticTest.java
View file
Expand Up @@ -34,18 +34,21 @@ public static void main( String[] args ) throws TaskException, IOException {

private static class AddTask implements Task {
private DoubleParameter p1;
private DoubleParameter p2;
private IntegerParameter p2;
private StringParameter p3;

public AddTask() {
p1 = new DoubleParameter( "first" );
p1.setPosition( 1 );
p1.setPrompt( "First number" );

p2 = new DoubleParameter( "second" );
p2 = new IntegerParameter( "second" );
p2.setPosition( 2 );
p2.setPrompt( "Second number" );
p2.setDefault( "0" );
p2.setIntDefault( 0 );
assertEquals( "0", p2.getStringDefault() );
p2.setStringDefault( "0" );
assertEquals( "0", p2.getStringDefault() );

p3 = new StringParameter( "comment" );
p3.setPrompt( "Comment" );
Expand All @@ -61,7 +64,7 @@ public Executable createExecutable( Environment env )
final String sval1 = p1.stringValue( env );
final String sval2 = p2.stringValue( env );
final double val1 = p1.doubleValue( env );
final double val2 = p2.doubleValue( env );
final int val2 = p2.intValue( env );
final PrintStream out = env.getOutputStream();
return new Executable() {
public void execute() {
Expand Down
2 changes: 1 addition & 1 deletion ttools/src/main/uk/ac/starlink/ttools/build/JyStilts.java
View file
Expand Up @@ -1186,7 +1186,7 @@ else if ( returnOutput ) {
* @param default value, suitable for insertion into python source
*/
private String getDefaultString( Parameter param ) {
String dflt = param.getDefault();
String dflt = param.getStringDefault();
boolean isDfltNull = dflt == null || dflt.trim().length() == 0;
boolean nullable = param.isNullPermitted();
if ( nullable || ! isDfltNull ) {
Expand Down
2 changes: 1 addition & 1 deletion ttools/src/main/uk/ac/starlink/ttools/build/UsageWriter.java
View file
Expand Up @@ -105,7 +105,7 @@ public static String xmlItem( Parameter param ) {
.append( "</code></dt>\n" )
.append( "<dd>" )
.append( param.getDescription().toString() );
String dflt = param.getDefault();
String dflt = param.getStringDefault();
if ( dflt != null && dflt.length() > 0 ) {
sbuf.append( "<p>[Default: <code>" )
.append( dflt.replaceAll( "&", "&amp;" )
Expand Down
2 changes: 1 addition & 1 deletion ttools/src/main/uk/ac/starlink/ttools/cea/CeaParameter.java
View file
Expand Up @@ -49,7 +49,7 @@ public CeaParameter( Parameter taskParam ) {
name_ = taskParam.getName();
description_ = taskParam.getDescription();
summary_ = taskParam.getPrompt();
dflt_ = taskParam.getDefault();
dflt_ = taskParam.getStringDefault();
isNullPermitted_ = taskParam.isNullPermitted();
type_ = "text";
if ( taskParam instanceof OutputTableParameter ||
Expand Down
2 changes: 1 addition & 1 deletion ttools/src/main/uk/ac/starlink/ttools/cone/ConeErrorPolicyParameter.java
View file
Expand Up @@ -40,7 +40,7 @@ public ConeErrorPolicyParameter( String name ) {
.append( "<n>" );
setUsage( ubuf.toString() );
setPrompt( "Action on cone search failure" );
setDefault( ConeErrorPolicy.ABORT.toString() );
setStringDefault( ConeErrorPolicy.ABORT.toString() );
setDescription( new String[] {
"<p>Determines what will happen if any of the individual cone",
"search requests fails. By default the task aborts.",
Expand Down
6 changes: 3 additions & 3 deletions ttools/src/main/uk/ac/starlink/ttools/cone/ConeSearchConer.java
View file
Expand Up @@ -96,7 +96,7 @@ public ConeSearchConer() {
} );

believeemptyParam_ = new BooleanParameter( BELIEVE_EMPTY_NAME );
believeemptyParam_.setDefault( "true" );
believeemptyParam_.setBooleanDefault( true );
believeemptyParam_.setPrompt( "Believe metadata from empty results?" );
believeemptyParam_.setDescription( new String[] {
"<p>Whether the table metadata which is returned from a search",
Expand Down Expand Up @@ -378,7 +378,7 @@ public void configureParams( Parameter srParam ) {
/* SIZE = 0 has a special meaning for SIA: it means any image
* containing the given image. This is a sensible default in
* most cases. */
srParam.setDefault( "0" );
srParam.setStringDefault( "0" );
srParam.setNullPermitted( false );
}

Expand All @@ -390,7 +390,7 @@ public ConeSearcher createSearcher( Environment env, String url,
final boolean believeEmpty,
StarTableFactory tfact )
throws TaskException {
formatParam_.setDefault( "image/fits" );
formatParam_.setStringDefault( "image/fits" );
String format = formatParam_.stringValue( env );
return new SiaConeSearcher( url, format, believeEmpty, tfact ) {
@Override
Expand Down
6 changes: 3 additions & 3 deletions ttools/src/main/uk/ac/starlink/ttools/cone/JdbcConer.java
View file
Expand Up @@ -62,7 +62,7 @@ public JdbcConer() {
new ChoiceParameter<AngleUnits>( "dbunit", AngleUnits.class );
dbunitParam_.addOption( AngleUnits.DEGREES, "deg" );
dbunitParam_.addOption( AngleUnits.RADIANS, "rad" );
dbunitParam_.setDefault( "deg" );
dbunitParam_.setStringDefault( "deg" );
dbunitParam_.setPrompt( "Units of ra/dec values in database" );
dbunitParam_.setDescription( new String[] {
"<p>Units of the right ascension and declination columns",
Expand Down Expand Up @@ -122,7 +122,7 @@ public JdbcConer() {
"A value of \"<code>*</code>\" retrieves all columns.",
"</p>",
} );
colsParam_.setDefault( "*" );
colsParam_.setStringDefault( "*" );

whereParam_ = new StringParameter( "where" );
whereParam_.setUsage( "<sql-condition>" );
Expand All @@ -149,7 +149,7 @@ public JdbcConer() {
"false (the default); on others it may be faster, who knows?",
"</p>",
} );
prepareParam_.setDefault( "false" );
prepareParam_.setBooleanDefault( false );
}

/**
Expand Down
12 changes: 6 additions & 6 deletions ttools/src/main/uk/ac/starlink/ttools/cone/SkyConeMatch2.java
View file
Expand Up @@ -115,7 +115,7 @@ public String stringToObject( Environment env, String sval ) {
}
}
};
modeParam_.setDefault( "all" );
modeParam_.setStringDefault( "all" );
modeParam_.setPrompt( "Type of match to perform" );
modeParam_.setDescription( new String[] {
"<p>Determines which matches are retained.",
Expand Down Expand Up @@ -163,7 +163,7 @@ public String stringToObject( Environment env, String sval ) {
"which covers VizieR and a few other cone search services.",
"</p>",
} );
usefootParam_.setDefault( Boolean.TRUE.toString() );
usefootParam_.setBooleanDefault( true );
paramList.add( usefootParam_ );

nsideParam_ = new IntegerParameter( "footnside" );
Expand Down Expand Up @@ -193,7 +193,7 @@ public String stringToObject( Environment env, String sval ) {
copycolsParam_ = new StringParameter( "copycols" );
copycolsParam_.setUsage( "<colid-list>" );
copycolsParam_.setNullPermitted( true );
copycolsParam_.setDefault( "*" );
copycolsParam_.setStringDefault( "*" );
copycolsParam_.setPrompt( "Columns to be copied from input table" );
copycolsParam_.setDescription( new String[] {
"<p>List of columns from the input table which are to be copied",
Expand All @@ -211,7 +211,7 @@ public String stringToObject( Environment env, String sval ) {

distcolParam_ = new StringParameter( "scorecol" );
distcolParam_.setNullPermitted( true );
distcolParam_.setDefault( "Separation" );
distcolParam_.setStringDefault( "Separation" );
distcolParam_.setPrompt( "Angular distance output column name" );
distcolParam_.setUsage( "<col-name>" );
distcolParam_.setDescription( new String[] {
Expand All @@ -226,7 +226,7 @@ public String stringToObject( Environment env, String sval ) {
paramList.add( distcolParam_ );

parallelParam_ = new IntegerParameter( "parallel" );
parallelParam_.setDefault( "1" );
parallelParam_.setIntDefault( 1 );
parallelParam_.setPrompt( "Number of queries to make in parallel" );
parallelParam_.setUsage( "<n>" );
parallelParam_.setMinimum( 1 );
Expand Down Expand Up @@ -269,7 +269,7 @@ public String stringToObject( Environment env, String sval ) {
paramList.add( erractParam_ );

ostreamParam_ = new BooleanParameter( "ostream" );
ostreamParam_.setDefault( "false" );
ostreamParam_.setBooleanDefault( false );
ostreamParam_.setPrompt( "Whether output will be strictly streamed" );
ostreamParam_.setDescription( new String[] {
"<p>If set true, this will cause the operation to stream on",
Expand Down
2 changes: 1 addition & 1 deletion ttools/src/main/uk/ac/starlink/ttools/join/JoinTypeParameter.java
View file
Expand Up @@ -45,7 +45,7 @@ public JoinTypeParameter( String name ) {
"</p>",
} );
setPrompt( "Selection criteria for output rows" );
setDefault( "1and2" );
setDefaultOption( JoinType._1AND2 );
}

/**
Expand Down

0 comments on commit 4850722

Please sign in to comment.