Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Proposal] Switch Region #34

Open
sebllll opened this issue Nov 25, 2020 · 5 comments
Open

[Proposal] Switch Region #34

sebllll opened this issue Nov 25, 2020 · 5 comments
Labels

Comments

@sebllll
Copy link

sebllll commented Nov 25, 2020

For #14 we could use the same regions that are used in other situations like TryCatch:

image

Pros

  • same syntax as in other regions
  • visually clear. the use of horizontal space makes sense in a general vertical oriented graph

Cons

  • might get messy with large switch-case regions with many conditions
  • regions with "many colors" are tedious to use at the time of this writing. (needs manual asigning of each node or iobox)
@YanYas
Copy link

YanYas commented Dec 31, 2020

I like this idea for smaller examples, but for larger ones I would turn to a dictionary. Dictionaries are a little more complicated to setup, though.

String Switcher as Dictionary

How would you add extra conditions?

@sebllll
Copy link
Author

sebllll commented Jan 5, 2021

well, this is just one proposal for the quest #14. and this proposal only focusses on how a switch-case could face the user.
internally this visual representation should translate to a switch-case - or even to some if statements (iirc switch-case is also just syntax sugar for lot of if statements).

@YanYas
Copy link

YanYas commented Jan 18, 2021

Fair play, I think it would be more readable. And I just came across a situation where I think something like it could be handy:
OnnxFromScratch6-TryTheseRuntimes-GPU Acceleration

I'm trying to create a 'fallback' system that prioritizes one option but will pick the next if there is an error. I suppose the internal logic a bit different though.

What I wonder is what would be the best way to add the new pins/cases to the node without it meaning a lot of extra clicks. This may mean a UX expansion to a ad not only one pin but a several at once.

@gregsn gregsn added the proposal label Mar 3, 2021
@gregsn
Copy link
Member

gregsn commented Feb 4, 2022

Your proposal is very much like what we always had in mind: moments for all of the cases. However, we didn't have in mind that you'd still need to check via a = operation since the moment already represents the case and is named by it.
If you look inside the ADSR patch you can see this region:
grafik
It basically illustrates what we had in mind.

A nice to have would be output border control points in order to output not just one value. We once discussed an IfElse region - which should be very similar to an enum switch with two cases - that allows writing into the output border control points by linking to that BCP from both moments: true/false or if/else.
Being able to link into output BCPs from different moments is touched here: #53
That said, with a bigger switch statement this way of expression could again get visually annoying, especially if you have more than one BCP: crossing links...

And then there are still issues in terms of readability and workflow. If the = and the ioboxes are missing then you need to hover all cases or put comments. And it is indeed still hard to patch in those type of regions, but you could argue that these should get fixed anyway.

Still not sure about the ideal solution.

Meanwhile, please have a look at #55 as a workaround.

@gregsn
Copy link
Member

gregsn commented Feb 4, 2022

I like this idea for smaller examples, but for larger ones I would turn to a dictionary. Dictionaries are a little more complicated to setup, though.

regarding the Dictionary approach: thanks for bringing it up! I just stumbled upon the Cons [Dictionary] again: that way they are not so hard to set up. see #55 Let's discuss this approach over there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants