anqitong / LangageDessinVectoriel

1 stars 2 forks source link

Struct/46 new SVGFile #48

Closed starsasumi closed 9 years ago

starsasumi commented 9 years ago

48

Really refactor our codes and remove lots of unnecessary interfaces and classes

starsasumi commented 9 years ago

I've moved SVGFile to xml package, sorry about that. I am sure about other parts. We only need svg in xml presentation (in this project xml == svg). And we DO NOT need to separate different small tasks only for write that pieces of xml. As for the creators, I leave them there only because they are already there.

starsasumi commented 9 years ago

@Thomas-dot-G SVG file is xml, and it's ONLY ONE TYPE OF PRESENTATION. Why we want to write a svg file that is not xml file?

And for god sake, the SVGFile is the XML version of the presentation, it should know all about xml. SVGFile would be the conductor if the painting is a music and svg/xml is an orchestra. In this example, the other possible presentation could be CD. Should CD know playing musical instrument (write SVG), no.

anqitong commented 9 years ago

SVG consists in an extension of XML which means its content is written in XML format : https://fr.wikipedia.org/wiki/Scalable_Vector_Graphics So from that point of view I agree with @starsasumi
BUT the class SVGFile is part of the model because we created it to support the presentation, so it is not purely presentation. As Thomas said, either the package xml contains all the classes where we find xml, either only the classes that displays

Thomas-dot-G commented 9 years ago

@starsasumi I was a little lost with your comparisons... But I mean, we need not to duplicate the code. If we want another representation of an image with a file, (ex text, why not ?), we will write again a very similar method to check if the file exist, add a name, create the file and write in it...

anqitong commented 9 years ago

And because we follow a Model View Controller architecture, it is more logical to put it in Model.

But it is also possible to consider that presentation/xml contains all the models related to purely displaying. So indeed, for me either in model, either in presentation/xml and that package should contains everything related to xml AND presentation (stuff thats has nothing to do with the components for the drawing such as pencils, colors ...)

starsasumi commented 9 years ago

@Thomas-dot-G the answer of your why is "we don't know yet", so we do not over engineer. Up till now, the other presentation we need is

La traduction produit des instructions en Java. On parle alors de langage embarqué. L'interprétation de ses instructions est définie par leur implémentation, typiquement regroupée dans une bibliothèque. Par exemple, la traduction pourrait s'appuyer sur la classe java.awt.image.Raster, qui fournit les fonctionnalités utiles pour le dessin matriciel ("raster graphics"). Plus simplement, elle pourrait s'appuyer sur la classe (abstraite) java.awt.Graphics2D, qui fournit des fonctionnalités utiles pour le dessin vectoriel ("vector graphics").

which share nothing but generic shapes with the existed SVGFile. This presentation even don't write file.

And if I agreed with you to keep a class to write file, we find out that java have done this:

java.io.BufferedWriter
java.io.File
java.io.FileWriter
java.io.IOException

the whole java.io package is already abstract way to read/write files. We do not even know the next file type we want is pure text or binary, there is truly no much codes could be shared. Also, SVGFile was not named by me. If you'd like to have an utility class to handle files, I would be more than happy. Just create another class and implement it. Then we can modify SVGFile class to use that new class to write files.

starsasumi commented 9 years ago

@anqitong in the UML, all xml related codes are put in model.presentation(.xml). Do you like that way? If no, could you give me more details about your packages suggestion ;). I would move them in the next commit.

anqitong commented 9 years ago

@starsasumi : model.presentation seems good to me

anqitong commented 9 years ago

@starsasumi :

the whole java.io package is already abstract way to read/write files. We do not even know the next file type we want is pure text or binary, there is truly no much codes could be shared. Also, SVGFile was not named by me. If you'd like to have an utility class to handle files, I would be more than happy.

it was named as SVGFile by me but it would be great to create a package src/util where we can put all these kind of files. I also suggest to make SVGFile extends java.io.File. Because it will be better to handle the deletion of the SVG in the future for example. Was suggested in issue #42

starsasumi commented 9 years ago

OK, all thing get done. Sorry for too aggressive tonight.

starsasumi commented 9 years ago

Should I merge or I need to modify somethings?

Thomas-dot-G commented 9 years ago

if you both agree with that, ok. The code itself is good, (even if there is a lot of changes for tab/4spaces).

I just have a question, this is just a question: why is the xml package in model ?

starsasumi commented 9 years ago

Top level MVC usually points to logic codes, uses interfaces and codes to hook M&V up. And that's why most our codes for now is in model. And in model package we have a sub-MVC, shape is model, xml file is view, and presentation&shapeState are controllers.

I should reuse tab for now to avoid unnecessary changes, and when we want, we change them to spaces in another issue.

anqitong commented 9 years ago

Don't bother yourself with indentation. If you do Ctrl + A and then Ctrl + I, you indent everything automatically.

starsasumi commented 9 years ago

@anqitong so, I merge now?

anqitong commented 9 years ago

Yes