Replacing big if else with regular expressions about roboxslicerextension HOT 28 CLOSED

Yes I managed to rewrite perl to Java like it was.
I specially love the assignments in conditional expression :)
There is probably a better solution like you are proposing.

The second one looks more robust

from roboxslicerextension.

Oh - you did good job converting it in nice tight code... But to me assignments in conditional expression are really screaming code smell... LOL

I'll try to re-organise it in a way of introducing bit of both and create separate pull request for you to check if you like it. I am not insisting on such solution - just curious if we can make it more... I don't know, readable? Logical? If that solution ended up neither - than we'll just abandon it.

And - there'll be more code like that for other slicers so it should be quite generic...

from roboxslicerextension.

By the way, just to throw a fly in the ointment here, I've managed to rewrite the postprocessor so that most of those regular expressions aren't needed.

It's just fleshing out the G1_Disabled code that's there at the moment. It seems to work OK, I just need to do a bit more testing. I think it's far more robust than all those regular expressions that seem quite fragile. It should actually work with other slicers as well, provided we can get the layer number hints working.

from roboxslicerextension.

Oh, don't worry - I've got time :) We'll do it at the appropriate point when it doesn't affect you doing real code...

from roboxslicerextension.

@natdan I was joking on the assignment in tests of corse it's a really bad usage.

@nebbian good news I was starting to do that yesterday until I saw natdan's code re-organisation

@nebbian I let you push your changes when you have time.

I'm working on a absolute to relative E convertion to handle the cura 2.5 code...

from roboxslicerextension.

@Benraay you've got me there! LOL
Please guys - do let me know if you feel I'm under your feet. I can refrain pushing some changes until you do yours... Or - if you believe there there's a good strategy how to go through first big bundle of code into @nebbian repo so we can easily work on our branches without tripping each other - do let me know! :)

from roboxslicerextension.

@natdan it's okay for me you make good improvements in the workflow no matter If I need I will make a branch to make my changes.

I will concentrate on the Absolute to relative E for now

from roboxslicerextension.

I just tried to push my new code to Natdan's fork in a new branch, but got the following message:

~/Dropbox/Apps/3D/Software/Robox/RoboxSlicerExtension_NatDan> git push --porcelain --progress --recurse-submodules=check origin refs/heads/Feature-Generic-Slicer:refs/heads/Feature-Generic-Slicer
'Feature-Generic-Slicer' rejected (non-fast-forward)
Delta compression using up to 2 threads.
Total 4 (delta 2), reused 0 (delta 0)
remote: Resolving deltas:   0% (0/2)           
remote: Resolving deltas:  50% (1/2)           
remote: Resolving deltas: 100% (2/2)           
remote: Resolving deltas: 100% (2/2), completed with 2 local objects.        
error: failed to push some refs to '[email protected]:natdan/RoboxSlicerExtension.git'
Not all refs have been pushed.

Any ideas?

from roboxslicerextension.

No idea... You're trying to create new branch, right? Did you try it again?
Maybe try just simple
$ git push
and follow instructions if it complains that you didn't set upstream branch...
Or - use git gui! :)

from roboxslicerextension.

Yeah I was using Smartgit.

I used the same process, and made a new branch in my repo. Feel free to pull from upstream to see the new, simpler, code.

from roboxslicerextension.

Will do. Since you've invited me to collaborate in your repo - shell we just move all the code there and make PRs from feature branches to master there?

from roboxslicerextension.

Yep, I think keeping it all in the one repo would be a good way to go. Using pull requests within the same repo seems like a great way to ensure that all the i's are dotted and t's crossed. I've just invited @Benraay as well.

from roboxslicerextension.

@nebbian I've just pushed branch to your repo
@Benraay I am sure that there's git incantation to do it through git commands but I just did:

[branch "Initial-Flow-Code"]
        remote = upstream
        merge = refs/heads/Initial-Flow-Code

change - remote above from 'origin' to 'upstream' (given that you have already defined upstream). If not just add following, too:

[remote "upstream"]
        url = [email protected]:nebbian/RoboxSlicerExtension.git
        fetch = +refs/heads/*:refs/remotes/upstream/*

from roboxslicerextension.

Ok but I have just written a few lines of code If I just clone the Initial-Flow-Code and paste my changes it can also be ok or not ?

from roboxslicerextension.

Of course. Sometimes it is less hassle doing it the way you suggest! :)

from roboxslicerextension.

@natdan did you start the postProcessor class I want to rewrite latest perl version of @nebbian but it could be good to do it with the new structure you suggested.
I'm not sure how exactly you want to organize it, Can you just make a sample code.

from roboxslicerextension.

No, I didn't - weather was awful for coding (otherwise fantastic for barbecue). I'll do it tonight if you don't mind.

from roboxslicerextension.

Barbecue is great also ! I did it last week end !
No matter let me now when you have something.

from roboxslicerextension.

Just letting you guys know that I've just merged the new postprocessing code into the master branch. It seems to be working well, and for some reason it's producing better prints than the old, more complicated code, at least for one particular model I've been printing. This should be a much better candidate to start with when writing the Java port.

from roboxslicerextension.

Yes this code is more generic and maybe more efficient !
I will try to port this tonight and move the stuff in master class.
and also make some tests with cura 2.5 if I have some time.

We are going to have a nice plugin !

from roboxslicerextension.

OK. I've done something. It compiles but I didn't get to test it out yet. Do have a look at Slic3rPostProcessor (and impl AbstractPostProcessor). Also, there's new class LineProcessor. Do let me know if it makes sense! :)

from roboxslicerextension.

@natdan and @nebbian
I have ported the perl code from @nebbian to Java and moves all the stuff to Slicer Class because all this code is generic now.

@natdan I have read all of your implementation on the Pattern and line postProcessor.
I'm sorry but my feeling is that we loose the readability of the processor in that way it's probably much more "Java way of coding" but more difficult to change it.

It took me some time to understand how you managed this and there is a lot of code around to make it work with the patterns.

I don't know what you think about that but with the new perl implementation there are less lines of code, I have moved most of declarations outside tests.

We need to keep it simple and easy to maintain !

from roboxslicerextension.

@Benraay It is up to you. If you think it is not clean or easy to understand, i.e. intuitive, (not the mechanism behind it but the class that does the transformation) then leave it away.

Oh, BTW, ever so slightly related - all those need to be refactored to different packages, but we don't have to do it just yet. :)

from roboxslicerextension.

Yes we probably need to make some refactoring at the end.

I'm sorry for the work you made on the patterns but I prefer to keep it a bit more simple to understand.

I just pushed a new version and added a little intall bash script that copies the jar and the bash script.
for now the bash script is calling the jar with all the options, I found a better way to cleanup the parameters, I had to change some things in the java code also, like keeping .gcode at the end of the file if not Slic3r adds .gcode extension at the end.

I does not print for now because of some bug on post processing but all the process is working.

from roboxslicerextension.

Oh, don't worry. I'm sure I'll be able to make you like that other solution - you just need more time! ;)

However, main thing is that I don't want to be in the way until we have nice, stable working code. All of that pattern thing is extra stuff and not quite yet needed.

from roboxslicerextension.

lol ok I let you the time to convince me later...

from roboxslicerextension.

I believe this issue is resolved for the moment. I'm very happy with the new java code :)

from roboxslicerextension.

for the moment it's working nice !

from roboxslicerextension.