APP_RESOURCE_LIST_IMPORT_EXPORT_R1

[Astea]

wiki page: APP_RESOURCE_LIST_IMPORT_EXPORT_R1 - effort: 9d

[dido]

• Think its a good idea to add button inside resource palette that allows importing of all kind of resources. But instead of inserting them on the stage, it will be better to import them inside the book, without creating instances on the current page.
• I expect this will work in the same manner for all insert/import dialogs.
• Please confirm what will be achieved on this topics in this revision before continue with design. If there is a need change the analysis, otherwise add comment on the wiki page.
• Everything else its more than fine.

Analysis reviewed 4p (30m)

[nenko]

• Importing resources only will be added in later revision of the task.
• Yes, inserting will work in the same manner

[dido]

• Lovely :)

[meddle]

• Design related things from pap:
  • FileManager is not very nice name for me. It is too general.
  • Are you sure that the warnings about raw types should be fixed by putting a " @SuppressWarnings("unchecked") "
  • Actually you don't need to set the source class to AppMenuItem. Leaving it to a concrete subclass should also work and will reduce computation time
  • Having these filters defined in FileDialogInput is bad. It breaks the idea of extensibility for me.
  • Using "FileDialogInput.ANNOTATION_FILTER" to check for zip file is quite incorrect.
  • Wouldn't it be better if you make an extension point for file openers. Its interface could have something like {getFileFilter(), getKind()}
• Code related notes from meddle:
  • Your code have no spacing, bad identation and strange things in it... see the code in GoodCodeExamples and fix yours.
  • Change

    	if(files[i].toString().charAt(files[i].toString().length() - 1) == '~'){
    		//opened text files generate a temporary file, ending with ~
    		continue;
    	}
    	unsupportedFiles += files[i].toString() + "\n";
    

to

	if (files[i].toString().charAt(files[i].toString().length() - 1) != '~') {
		unsupportedFiles += files[i].toString() + "\n";
	}
	

• Don't swallow exceptions like this:

  	try {
  		zf = new ZipFile(zipFile);
  	} catch (Exception e) {
  		return null;
  	}
  

• Some editor's temporary files don't end with '~'... But that is not important :)
• hyphenate maximum at the 100th character of the line!!!
• In the tests:
  • You have no test for your new FileManager (rename it!) write good one, your new logic is there after all.
  • assertTrue(pwa.elementViews().get().size() == 7); change this line to assertTrue(7, pwa.elementViews().get().size());, that way they are better for readers.

• Overall:
  • We don't like that the FileManager dispatches ADD_NEW_FRAME. Imagine you import a group of resources or one resource only to the resource palette with dran and drop for example, we wouldn't be able to use your manager... A better aproach is to dispatch something like OPEN_FILE event and the logics for adding frames to listen to it and dispatch events... I think that is important.
  • Write good SVN comments when commiting like #TICKET_NUMBER TICKET_NAME {What did you change}

[meddle]

OK, it seems you took seriously our comments, so this design will pass now. But be aware of the following:

• Your code MUST run with the true runners, we will test it.
• Your code MUST not break anything that is working now (be careful with the book saving again)
• Don't write "implementation" in the design section, it's the wrong way at design to have implementation, which is your case... Write "design related code".
• For the implementation review provide all the needed revisions, because if your code is merged with compilation errors or warnings, it will fail...

4p (30m)

[meddle]

I pass it, but I didn't like some things in the code like hyphenation on comma and the fake javadod, especially the fake JavaDoc...

3p (80m)

[deyan]

Batch update from file query-obsoleted.csv