Result of Marietta Hamberger's research project #12
Conversation
…other files for build and launch.
| fileChooser.getExtensionFilters().addAll( | ||
| new ExtensionFilter("CSV files", "*.csv"), | ||
| new ExtensionFilter("TAB files", "*.tab"), | ||
| new ExtensionFilter("TSV files", "*tsv")); |
| <description url="http://www.example.com/description"> | ||
| [Enter Feature Description here.] | ||
| </description> | ||
|
|
||
| <copyright url="http://www.example.com/copyright"> | ||
| [Enter Copyright Description here.] | ||
| </copyright> | ||
|
|
||
| <license url="http://www.example.com/license"> | ||
| [Enter License Description here.] | ||
| </license> |
| [INFO] Scanning for projects... | ||
| [INFO] | ||
| [INFO] ------------< org.insilico:org.insilico.maven.dependencies >------------ | ||
| [INFO] Building InSilico - Maven Dependencies 1.0.0-SNAPSHOT | ||
| [INFO] --------------------------------[ pom ]--------------------------------- | ||
| [INFO] ------------------------------------------------------------------------ | ||
| [INFO] BUILD FAILURE | ||
| [INFO] ------------------------------------------------------------------------ | ||
| [INFO] Total time: 0.119 s | ||
| [INFO] Finished at: 2020-03-16T02:43:46+01:00 | ||
| [INFO] ------------------------------------------------------------------------ | ||
| [ERROR] Unknown lifecycle phase "p2". You must specify a valid lifecycle phase or a goal in the format <plugin-prefix>:<goal> or <plugin-group-id>:<plugin-artifact-id>[:<plugin-version>]:<goal>. Available lifecycle phases are: validate, initialize, generate-sources, process-sources, generate-resources, process-resources, compile, process-classes, generate-test-sources, process-test-sources, generate-test-resources, process-test-resources, test-compile, process-test-classes, test, prepare-package, package, pre-integration-test, integration-test, post-integration-test, verify, install, deploy, pre-clean, clean, post-clean, pre-site, site, post-site, site-deploy. -> [Help 1] | ||
| [ERROR] | ||
| [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch. | ||
| [ERROR] Re-run Maven using the -X switch to enable full debug logging. | ||
| [ERROR] | ||
| [ERROR] For more information about the errors and possible solutions, please read the following articles: | ||
| [ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/LifecyclePhaseNotFoundException |
| @@ -40,7 +39,8 @@ | |||
|
|
|||
| @Override | |||
| public Object compute(IEclipseContext context, String contextKey) { | |||
| System.out.println("Compute..."); | |||
| return IInjector.NOT_A_VALUE; | |||
There was a problem hiding this comment.
maybe change this to a "not implemented" Exception
There was a problem hiding this comment.
I gave up after about 1/3 of this PR.
Please make the code "human-readable":
Comment the code (most of the methods, constructors, etc. have no javadoc/comment at all)
Please be consistent in formatting
DRY
etc.
I would suggest general refactoring of the code. Please take a look at some coding guidelines (Oracle/Eclipse WiKi).
There are also some nice books touching these topics:
https://www.amazon.com/Refactoring-Improving-Design-Existing-Code/dp/0201485672
https://www.amazon.com/Clean-Code-Handbook-Software-Craftsmanship/dp/0132350882
| @@ -21,9 +18,11 @@ | |||
|
|
|||
| @PostConstruct | |||
| private void init(BorderPane parent) { | |||
| if (doc!=null) { | |||
There was a problem hiding this comment.
With this check added we assume that the doc can be null on this stage. Do we need to set an alternative error view for the case when the doc is not available or we inform the user in some different way?
| @@ -40,7 +39,8 @@ | |||
|
|
|||
| @Override | |||
| public Object compute(IEclipseContext context, String contextKey) { | |||
| System.out.println("Compute..."); | |||
| return IInjector.NOT_A_VALUE; | |||
| /*System.out.println("Compute..."); | |||
There was a problem hiding this comment.
Please remove redundant code or at least add a description of the potential code intention.
| String defaultDouble = "0.0"; | ||
| String defaultInt = "0"; |
There was a problem hiding this comment.
Please replace magic values with named constants. Also, they should be used in the following code.
| /* | ||
| * property value getter/setter | ||
| */ | ||
|
|
There was a problem hiding this comment.
Somehow the comment "ran away" from the method signature :) (please remove the blank line between the signature and the comment).
| } | ||
|
|
||
|
|
||
| /* |
There was a problem hiding this comment.
Please provide javadoc instead of multiple-line comment.
|
|
||
| public void setProperty(String attribute, String value) { | ||
| switch (attribute) { | ||
| case "id": |
There was a problem hiding this comment.
Please see the enum-related comment above.
| compartment.setSBOTerm(SBMLUtils.convertToSBO(sboTerm.getValue())); | ||
|
|
||
| compartment.setSize(SBMLUtils.convertToDbl(size.getValue())); | ||
| Kind u = SBMLUtils.convertToUnit(unit.getValue()); |
There was a problem hiding this comment.
Please change the variable name to the self-explaining one.
| if (value.matches("^[0-9]+\\.[0-9]+$")) { | ||
| value = value.split("\\.")[0]; | ||
| }else if (value.matches("^[0-9]+$")) { | ||
| System.out.println("int stays int"); |
There was a problem hiding this comment.
I am not sure if it is necessary to provide this comment. What does "int stays int" says to the user during the normal usage of the application? Probably, logging this information could make sense (in proper way)?
|
|
||
| public CompartmentTableView(){ | ||
| setEditable(true); | ||
| initExampleData(3); |
| private ObservableList<CompartmentInstance> instances = FXCollections.observableArrayList(); | ||
|
|
||
|
|
||
| public CompartmentTableView(){ |
There was a problem hiding this comment.
Please provide javadoc for public constructor.
This modification brings improvements to the SBTab plugin SBMLsheets which by @RobertDeibel initially created, and also fixes several problems with the core framework of InSilico.