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

Utility method to convert results to a hash #13

Open
DanRathbun opened this issue Sep 28, 2021 · 7 comments
Open

Utility method to convert results to a hash #13

DanRathbun opened this issue Sep 28, 2021 · 7 comments

Comments

@DanRathbun
Copy link
Contributor

DanRathbun commented Sep 28, 2021

Both Thomas and I has the same idea about getting hash results instead of array results. After looking at the situation ...

The simplest solution is to add a public utility method that the consumer(s) can pass the results array into.

This means there is no extra constructor flag argument needed, nor a special prompt(with_hash) flag argument, nor a special type_convert method argument that causes it to map values to a hash instead of an array.

This also means consumer(s) can use this utility method with the old UI::inputbox argument signature if they forgo the HtmlUI::inputbox wrapper class method and use the HtmlUI::Inputbox::new constructor instead:

window = HtmlUI::Inputbox::new(["Name","Age","Occupation"], ["","",""], "Personal Data ...")
results = window.prompt
data = window.hash_convert(results) if results

The utility method:

      # Returns a hash using the input labels as keys and the results as values.
      #
      #   window = HtmlUI::Inputbox::new(["Name","Age"], ["",""], "Personal Data ...")
      #   results = window.prompt
      #   data = window.hash_from(results)
      #
      # @param values [Array] The results array returned from the {#prompt} method.
      # @return [Hash] The resultant hash.
      # @raise [TypeError] If the `values` argument is not an Array.
      def hash_from(values)
        fail(TypeError,'Array argument expected.',caller) if !values.is_a?(Array)
        hash = {}
        @options[:inputs].each_with_index { |input, index|
          label = input[:label].to_s
          hash[label]= values[index]
        }
        hash
      end

The only thing I worry about is if blank empty labels get through.
Ruby Hash accepts empty string labels as valid so if more than 1 blank label existed data might be lost.

@DanRathbun
Copy link
Contributor Author

Decided that the method name should be #hash_from(values) as it is not really doing any converting.

Also I found that it is quite simple to add a "prompt wrapper" method to complement this one ...

      # Creates the inputbox dialog object, loads the properties and shows
      # the dialog window modally. Returns a hash of user values.
      #
      # @return [Hash] The resultant hash. Empty if user cancels.
      def prompt_for_hash
        results = prompt()
        results.empty? ? {} : hash_from(results)
      end

@thomthom
Copy link
Member

I was thinking prompt return a Hash with field if and values instead of an Array of values only. Not seeing the point of keeping prompt as-is.

@DanRathbun
Copy link
Contributor Author

Yes I thought of this also. You'd need the Array choice for UI::inputbox compatibility if it's a public library.

Basically prompt() can have hash: false argument and a conditional at the end.
I didn't propose it because I was trying hard not to change prompt().

I am also getting the feeling trying this out that prompt strings may be too long for hash keys.
I'm mulling over how another optional arg can be added to set the return hash key for each input.

This leads to another problem ...I've run into ...
Issue 13 : Input subclass constructors force use of positional arguments

@thomthom
Copy link
Member

You'd need the Array choice for UI::inputbox compatibility if it's a public library.

But that's something the HtmlUI.inputbox method can convert. That method is responsible for the compatibility shim.

I am also getting the feeling trying this out that prompt strings may be too long for hash keys.

You're right. And adding manually passed in IDs will then quickly make the interface cumbersome. Need to rethink the interface as a whole I think. For instance, I'm not happy about the constructor signatures for the various controls. I'm thinking default value should be a named argument and not positional. Might have to play around with some mock-code to explore alternatives.

@DanRathbun
Copy link
Contributor Author

You'd need the Array choice for UI::inputbox compatibility if it's a public library.

But that's something the HtmlUI.inputbox method can convert. That method is responsible for the compatibility shim.

I'm sorry but:

(a) I feel that then the ::inpubox class wrapper method would no longer be a "compatibility shim"; and ...

(b) it is quite easy (as I've shown) to write a wrapper method that returns a hash instead of an array. This also applies to class methods.
A new sibling class wrapper that runs the results through the "hashifier" method can be easily done, thus ...

    # @return [Hash,false]
    def self.inputbox_deluxe(*args)
      fail(ArgumentError,'No arguments given. (1..4 expected)',caller) if args.empty?
      # Minimum arguments allowed by `UI.inputbox` is 1 array with 1 prompt String.
      window = InputBox.new(*args)
      results = window.prompt
      results.empty? ? false : window.hash_from(results)
    end

... but (c) I do not think this is needed. Once the developer sees that they have more control by using the InputBox::new constructor I believe that they will forgo using the "compatibility shim".


And adding manually passed in IDs will then quickly make the interface cumbersome.

Oh I don't think so. You've seen my example in the "input control constructor arguments" issue.

I've modified the "hashifier" utility method to use input[:key] if it exists, and if not default to input[:label]

      # Returns a hash using the input labels as keys and the results as values.
      # If the input has a `:key` item then it is used for the hash key rather than
      # the `:label` item (which often is too long to serve well as a key.)
      #
      #   window = HtmlUI::Inputbox::new(["Name","Age"], ["",""], "Personal Data ...")
      #   results = window.prompt
      #   data = window.hash_from(results)
      #
      # @param values [Array] The results array returned from the {#prompt} method.
      # @return [Hash] The resultant hash.
      # @raise [TypeError] If the `values` argument is not an Array.
      def hash_from(values)
        fail(TypeError,'Array argument expected.',caller) if !values.is_a?(Array)
        hash = {}
        @options[:inputs].each_with_index { |input, index|
          label =( input[:key] ? input[:key] : input[:label] )
          # The labels can be empty strings, if so use index: 
          if !label || label.empty?
            hash[index]= values[index]
          else
            hash[label]= values[index]
          end
        }
        hash
      end

@DanRathbun
Copy link
Contributor Author

DanRathbun commented Sep 30, 2021

Oh ... and another thing occurred to me.

It is usually undesirable for a method to return multiple class types (besides false or nil to denote a failure.)

I actually did temporarily add a symbol argument to prompt to tell it what to return (:Array or :Hash.) I am rethinking this now.

I do still have the prompt_for_hash wrapper method.

But we could switch, make prompt return a hash instead.
And then only for the "compatibility shim" convert the hash back to an array thus ...

      def values_from(hash)
        fail(TypeError,'Hash argument expected.',caller) if !hash.is_a?(Hash)
        values = []
        @options[:inputs].each_with_index { |input, index|
          key =( input[:key] ? input[:key] : input[:label] )
          # The labels can be empty strings, if so use index: 
          if !key || key.empty?
            values << hash[index]
          else
            values << hash[key]
          end
        }
        values
      end

And then the last line of the "compatibility shim" method would be:

      results.empty? ? false : window.values_from(results)

@DanRathbun
Copy link
Contributor Author

DanRathbun commented Oct 1, 2021

But we could switch, make prompt return a hash instead.
And then only for the "compatibility shim" convert the hash back to an array thus ...

Just a quick note to say that in my edition, I went ahead and did this.

I did not change the JS in this respect, it still returns a values array to the "accept" callback.
This is important that it return an ordered list of values matching the order of the options[:inputs].

Within the "accept" callback if there are no errors after type conversion, it sets results equal to the converted values.

Then at the end of the prompt() method, it tests the results for emptiness and if not runs the results array through the hash_from() "hashifier" method as posted above. viz:

        results.empty? ? {} : hash_from(results)

I also gave the consumer the option of using a wrapper that returns false if the user cancels the dialog:

      # Creates the inputbox dialog object, loads the properties and shows
      # the dialog window modally. Returns a hash of user values or `false`
      # if the user cancels.
      #
      # This is an alias for: {#prompt_for_hash!}
      #
      # @return [Hash,false] The resultant hash; `false` if user cancels.
      def prompt!
        results = prompt()
        results.empty? ? false : results
      end

And for those consumers who still want array results I added wrappers for this:

      # Creates the inputbox dialog object, loads the properties and shows
      # the dialog window modally. Returns an array of user values.
      #
      # @return [Array] The resultant values array. Empty if user cancels.
      def prompt_for_ary
        results = prompt()
        results.empty? ? [] : values_from(results)
      end

      # Creates the inputbox dialog object, loads the properties and shows
      # the dialog window modally. Returns an array of user values or `false`
      # if the user cancels.
      #
      # @return [Array,false] The resultant values array; `false` if user cancels.
      def prompt_for_ary!
        results = prompt()
        results.empty? ? false : values_from(results)
      end

I'm of the mind to give the consumer what they want on their hamburger sandwich.

;)

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

No branches or pull requests

2 participants