BOOK_HTML_EXPORT_R0

[Astea]

wiki page: BOOK_HTML_EXPORT_R0 - effort: 0.5d

[mitex]

changed owner

[dido]

Currently there is a implementation of html export that could fit in this task, but it's not part of the trunk. It will be nice if we could reuse as much as possible of this.
Analysis (1h)

[deyan]

A requirement for PDF export is

• PDF export must be selected from menu File -> Export As... -> PDF

However, this was not implemented that way. I think this should be a requirement for this task as we don't want to have all of the export types in the menu. A submenu "Export As..." should be implemented. You should also mention what the limitations of the HTML export will be (kind of frames, rotated frames, audio, page borders, etc.)

[dido]

Updated according to the comments and test requirements added.
Analysis (1h)

[deyan]

Please document well what is exported correctly, what is not, and what are losses.

[meddle]

• Code related to the design problems:
  • You didn't add the new html module to the main pom.xml!
  • The code related to the design MUST be merge-able with the trunk! You have some errors, so it is not.
  • About the test:
    • Use the FileEntryManager to get your files!
    • @Befor/@After/@Test annotations are missing, see GoodCodeExamples.
    • "book.pages().get().get(0).get(Page.class);" that's ugly, what do you think about this : "book.currentPage().get().get(Page.class, book);"?
    • Whats that in the test -> "e.printStackTrace();" when catching an exception, use the loger and write "fail("message");".
    • Why you wrote "new String(htmlString)". If you want to see if two strings without the white spaces are equal use the String#replaceAll("
      s", "") method.
  • Some constant things like the ".html" in your logic MUST be extracted to constants!
  • It is good to write assertations like "assert index > -1;", but write them messages too.
  • If the design code comes with new module, MAKE SURE it builds with maven and it runs with the true reader and true author runners!

• Problems with the design:
  • Now we have "export to PDF" file->menu item, you must remve it and add only "export..." item which will contain the both "export to PDF" and "export to html" items, see the analysis requirements.
  • Write in the analysis, if not in the design your new shortcuts.
  • If you will keep all the html persisters in the extra.html module, you must write why, because the Persisters from the Persistence module are divided into their accurate modules, for example the ImagePersister is in the image module.
  • Write why you can not use the Persisters like Book->Storage from the Persistence module, the documentation must point why you can not reuse something already done. Write the differences in your use case.
  • One UML diagram should be in help, but it's not mandatory.

2p (75m)

[peko]

• Fixes and comments about the code:
  • Thanks for the comments! :-) Everything fixed as proposed except for:
    • The FileEntryManager is not suitable for test in my opinion. I do not use already created files and stuff, but rather create a file read it and then delete it. Correct me if I am wrong.
    • "book.pages().get().get(0).get(Page.class);" is not ugly, but is the right way to get a page from the model. What you propose is something to be announced as null and void and that had already been discussed. Same applies to page.currentFrame... What will happen if we have several applications dealing with one book(collaboration). Current things will become part of the view. These two methods should have already been deprecated but are not :-/.
    • About the exception, the logger has a method fatal("..."), not fail, but I suppose a better thing is to wrap and throw the exception. FileNotFoundException speaks better for itself instead of a message.
    • I do not see anything bad in new String(htmlString) and I do note get the idea of that comment. The "new String(htmlString)" has nothing to do with spaces. It creates a String from a StringBuilder. Replacing the white spaces as proposed will even require more resources instead of comparing the strings with the white spaces.

• Fixes and comments about the design:
  • Menus fixed. Titles, mnemonics and stuff explained in the design. This includes the changes around menus that are not part of this task.
  • Persisters and things about them fixed in the design section. I do not think that the purpose of the new persisters should be expained. After all generally speaking persisters are defined by their schema. If the schema differs there is no need to explain why a persister cannot be reused. Small explanation given though.
  • Diagram will not be added, since I do not find it useful for the current number of persisiters. Maybe on later revision when we have more persisters a diagram will be useful.

• Thanks for the comments. Maybe we should discuss some of the things for which we have different opinion.

[meddle]

The design is fine now, but why are you giving me sync-trunk revisions?? They of course are not merge-able with the trunk, you had to give me code that do not cause errors with the trunk... I fixed it... You have big problem with putting spacings it's a random matter...

1h (50m)

[meddle]

:D I'm sleepy... 3p (50m) :D:D

[pap]

I want to note that the menu should no be "Export as..." but "Export as". The "..." is used to mark that when the item is pressed a modal dialog will appear.

[peko]

That is something I did not know :-)

[meddle]

The implementation fails because:

• The test don't pass. The reason is there is no contents for the frames... Make sure that the test tests both image and text contents and then give it to review it again!
• You have bad spacing (again), but that is not a reason to fail the implementation.
• Bad way of getting a resource from it's resource ref:

  HotTextBookResource textResource = 
  	hotTextContent.mainResource().get().get(
  		HotTextBookResource.class);
  

  Use the frame parent of the content, if the path is ralative the above thing may not work...
• Your Persisters can only save, so assert that other mode is impossible... Defensive programming man...

2.5p (35m)

[peko]

Thanks the comments... Everything done as proposed. :-)

[meddle]

The implementation is ok now :)

3.5p (80m)

[deyan]

Batch update from file query-obsoleted.csv