OOP Refactoring

[szaghi]

OOP Refactoring started.

New branch created for this aim feature/OOP-refactoring.

VTK file class

A new class has been created.

It is not complete, but already huge... The include statements have been exploited to sanitize implementation.

@victorsndvg Dear Victor, as soon as you can/want see this new refactoring I would like to know your comments.

[victorsndvg]

Hi @szaghi ,

i'm happy to see this issue open!

First of all, congratulations for this work, I was taking a look over the code and now is more easy to read!

I hope to have some time to spend in this project as soon as possible and be able to help you with anything you need. I have to study a little the implementation of the readers to try to adapt them to your new code.

Finally, I'm agree with you that the code is really huge, and the use of include help to organize the code, but I was expecting to see the name of the procedures in vtk_file_class file instead of looking for them in the *_method.inc files. I think that having the procedure names in the body of vtk_file derived type helps to give more lights in the first sight. What do you think?

[szaghi]

Dear @victorsndvg,

thank you for your feedback.

I hope to have some time to spend in this project as soon as possible and be able to help you with anything you need. I have to study a little the implementation of the readers to try to adapt them to your new code.

No rush, do not worry. For the Importers, I am trying to update concurrently FoXy to sanitize the XML IO parts...

Finally, I'm agree with you that the code is really huge, and the use of include help to organize the code,

I started using submodules but I faces with some issues (probably due to immature compilers support, or more likely due to my inexperience with them), so I had to revert back to old includes...

I was expecting to see the name of the procedures in vtk_file_class file instead of looking for them in the *_method.inc files... I think that having the procedure names in the body of vtk_file derived type helps to give more lights in the first sight. What do you think?

I totally agree, I have tried to obtain a trade-off putting them into comments, but this is unsatisfactory. I will try to improve this aspect.

Thank you for help, it is very appreciated.

[victorsndvg]

Dear @szaghi ,

I've been thinking a little in the OO design for Lib_VTK_IO and I have some suggestions. Let me know your thoughts!

Based on the XDMF library (and also XH5For :) ), one could isolate 2 main responsibilities in the tasks performed by Lib_VTK_IO. The first one, the XML/tags IO, and the second one the raw data IO (raw, binary, appended data).

In a first sight, I think that the object in charge of XML reading/writing will be unique (it will always write tags with more or less attributes) , but it will not happen the same with the object in charge of manage the raw data (coordinates, connectivities and fields).

Currently, when writing the raw data it is always needed a select case to perform the right action. Why not to have a 3 data writers (ascii, base64, binary) that could be dynamically allocated (State pattern) at the initialize() step?

If one follow the previously explained State pattern, another suggestion could be the use of a Decorator pattern. It seems that the ascii and base64 readers/writers performs always the same actions but with a different stream, if this is true, this objects could be lightweight wrappers of a common/shared derived type with the single responsability of converting the data to the right stream format.

I'm not totally sure about this words, and probably I misunderstood some things... but I think that to talk about the design consideration could be positive.

Hope it helps!

[victorsndvg]

Basic scheme of the ideas in the previous comment

[szaghi]

Dear @victorsndvg

Wow! I think I almost agree on all, but today has been a complicated day... I have not the time completely understand all your points an replay adequately. Tomorrow I will do it.

Thank you very much, this is very helpful!

[szaghi]

Dear @victorsndvg

I finally studied some about state and decorator patterns... yes, will go with them!

I hope to lint some files today and then I will try to go on state-decorator way!

Great suggestion!

[victorsndvg]

Good news @szaghi ,

I will be aware of the Lib_VTK_IO changes. Be free to ask anything, i'm open to help you!

[szaghi]

@victorsndvg

Let us consider the "kind" of writing possibilities

Codec	local position	appended position
ascii	:white_check_mark:	:no_entry_sign:
binary (base64)	:white_check_mark:	:white_check_mark:
raw	:no_entry_sign:	:white_check_mark:

Do you think we should start with 4 write states?

E.g.

• write_ascii_local_state
• write_binary_local_state
• write_binary_appended_state
• write_raw_appended_state

[victorsndvg]

@szaghi ,

I'm not sure at all if this is the best approach... it seem to be a good start point. After implementing it, we will probably check all the occurrences between different strategies. Maybe at the end we will have implemented 2 hierarchy levels (Appended/Format or Format/Appended) depending on the common variables/procedures.

Other suggestion:

I'm not totally sure and is not really importat, but another thing I've found that can be "fixed to be more consistent" that is related with the scratch unit variable, only used in the appended cases.

Why not to use always 2 units? One for the XML data and the other for the RAW data. It both units are the same we will be in the LOCAL case, if not in the 'APPENDED' case.

Best!

[szaghi]

@victorsndvg I have played with state/strategy patterns... due to my inexperience it seems very complicated... Do you want to see the first skeleton? I can push on OOP branch these unused classes...

[victorsndvg]

@szaghi ,

Ok, i can take a look to this new code if you want! :)

[szaghi]

@victorsndvg pushed.

[victorsndvg]

@szaghi ,

I'm not an expert and probably I will be wrong in a lot of things, but let's go!

I'm not able to completely understand where this piece of code is inside all the hierarchy. Who is the context and who are the states? Finally, you will also need a hierarchy for the XML IO?

I think that the state is the one in charge of allocate/deallocate the polymorphic objects (setting the state). As a starting point, the simplest thing is to have a non polymorphic context (other case is too complicated for me and I cannot imagine a case of use.. that of course exists!). In your implementation, the derived type in charge of finally setting the state (set_state) is extending from another one, is this really needed? is this class the context?

The second doubt is related with how you are setting the state. You are using an object as input to copy it to the current state. I think that this is a really advanced way to interact between some software subsytems (I only see it in the sparse matrix of Rouson). What i'm not able to understand in this point is, will you expose the different concrete objects to the user? our you will implement an extra layer over the library and the user will only use flags/strings (like in Lib_VTK_IO)?

If you want to take a look, I've implemented something similar in XH5For. Probably is not the best design ... but I'm thinking in a similar implementation.

You can see my "context" here . I'am using (toy) abstract factories to create the polymorphic objects in the initialize() procedure.

And I have several derived types that are acting as states. For example, you can take a look inside the directories below: My XML handler My RAW handler

[szaghi]

@victorsndvg Wow, I have to study a lot...

What do you think about strategy instead of state pattern? At a first view it seems simpler for a poor donkey like me and still suited for our aims.

[victorsndvg]

@szaghi ,

Ufff difficult question for me ... I'm not sure if in my code i'm using state or strategy :laughing:

both are similar patterns, I think that the class diagram is identical in both cases. For me the most important difference is semantic. Strategy pattern is made to wrap (single purpose?) algorithms in a black box, while State is made to dynamically change the way an (more complex?) object ask to the same request.

:thought_balloon: (almost the same meaning)

I think that the Rouson sparse matrix is a nice example for State pattern. A really small explanation: He has some implementations of sparse matrix (COO, CSR, etc.) that are used in different stages of the lifecycle of a problem. He uses COO while building the matrix (inserting entries) and then convert this matrix, in a transparent way for the user, to another format to operate with it. Matrix will change dynamically but the client will never appreciate the underlying complexity!

An example of application of Strategy pattern could be, for example, if you implement some integration methods under a common abstract class, and the context (object who will perform integration) receives the object and apply it.

Finally, I suppose that the Strategies are simpler objects than States. Maybe i'm wrong ...

Probably a better example of Strategy pattern in java: http://www.tutorialspoint.com/design_pattern/strategy_pattern.htm

In the following post some people discusses about the different between State and Strategy. http://stackoverflow.com/questions/1658192/what-is-the-difference-between-strategy-design-pattern-and-state-design-pattern

Hope it helps!

[szaghi]

@victorsndvg

Agree on all, and all help!

Thank you!

[szaghi]

@victorsndvg

It seems that I am more confident with the strategy pattern: in the last push to the OOP branch there is the almost complete xml_writer_ascii_local strategy, namely the ascii writer facility. This looks to me like a good trade-off between efficiency and clearness. What do you think about?

Note that probably the encode facility will be trimmed out and put into StringiFor so the final xml_writer_ascii_local should be a little more concise.

[victorsndvg]

Sorry @szaghi ,

we are on holidays and I cannot take a look now. I will write you on Monday!

Enjoy the weekend!

[szaghi]

Dear @victorsndvg

do not worry!

In the last push on OOP branch I have completed the transition to the OO Strategy Pattern. In particular we have:

• xml_writer_asbtract, the abstract strategy:
  • xml_writer_ascii_local, the concrete strategy to write in ascii local format;
  • xml_writer_binary_local, the concrete strategy to write in binary local format;
  • xml_writer_appended, the concrete strategy to write in append position in both raw or binary format (I have not split this into two strategies, say xml_writer_binary_appended and xml_writer_raw_appended, because they differ by just two if into two methods...).

These writer strategies are used into the 3 file classes presently implemented, namely:

• pvtk_file
• vtk_file
• vtm_file

that are the only objects exposed by the main front-end module, thus we are already thread/process-safe!

Before moving to implement the xml_reader facilities I will update some _thirdparty libraries:

• FoXy will get all the agnostic tag-related methods that presently are here;
• StringiFor will probably get the arrays encoders that presently are here.

After that I will implement your readers!

Thank you very much for pointing me to the strategy pattern, it has been very convenient to adopt.

[szaghi]

P.S. I am going to change the name of the library, I prefer

VTK_Fortran

do you agree?

[victorsndvg]

Perfect for me! :+1:

[szaghi]

FoXy is almost ready, soon I will start to use it for this OOP refactoring branch.

[victorsndvg]

Dear @szaghi ,

congratulations, new code is amazingly simplest to read! :+1:

I was also taking a look to FoXy. I have seen that you already implemented the writing capabilities. Great work! I will try to check it as soon as possible

[szaghi]

Victor you are too much kind, but the idea is your, I have made only some oompa loompa stuf :smile:

[appleparan]

How about removing underscore to new library name? I didn't see any underscores in library name. It could be wired when link with -lVTK_Fortran. How about VTKFortran?

[szaghi]

Dear Liam, right observation. During the next week I hope to push the new release called VTKFortran. Il 01/lug/2016 04:24, "Liam Jongsu Kim" notifications@github.com ha scritto:

How about removing underscore to new library name? I didn't see any underscores in library name. It could be wired when link with -lVTK_FortranHow about VTKFortran?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.