2
\$\begingroup\$

Wrote this for getting the content of image from the given URL. It accepts a URL and returns an array of shape number_of_channels image_height image_width. Each "row" represents a pixel, and each element is within the range of 0 to 1.

  • What are some issues with my implementation?
  • Is there a better way to turn the data from HttpCommand.Get into image?
  • Is there a better way to handle different image format?
  • How to make it more portable?

 

∇ r←GetImg url ;ns
  'ns'⎕NS'url'
  :With ns
    ⎕SE.SALT.Load'HttpCommand'

    valueOf←{⊃(,¯1↑⍉⍵)/⍨(⊂⍺)≡¨,1↑⍉⍵}
    split←{⍵⊆⍨~⍺=⍵}

    tmp_path←(⊢2 ⎕NQ # 'GetEnvironment',⊂)'tmp'

    res←HttpCommand.Get url

    (type fmt)←'/'split'Content-Type'valueOf res.Headers
    tmp_ft←(tmp_path,'\tmp.',fmt)(⎕NCREATE⍠('Unique' 1))0
    tmp_name←⊃⎕NINFO tmp_ft
    res.Data ⎕NREPLACE tmp_ft 0
    ⎕NUNTIE tmp_ft
    'bm'⎕WC'Bitmap'('File'tmp_name)
    ⎕NDELETE tmp_name
  :EndWith
  r←255÷⍨256(⊥⍣¯1)ns.bm.CBits
∇ 
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$
  1. Your use of :With is a clever way to avoid having to localise names, but you should be aware of the peculiarities of :With to avoid frustration in the future. Even inside the :With block, you can access r and in this case also url, but if the argument was an object, you would have to mention it explicitly, so you're doing the right thing here. Since you can access r, you can move the last line inside the block:

    r←255÷⍨256(⊥⍣¯1)bm.CBits

  2. You do not need the ns local, as you can give :With an anonymous namespace:

    :With ⎕NS'url'

  3. Since you're only using HttpCommand once, you can create an anonymous instance of it, which is then used (and discarded) immediately:

    res←(⎕SE.SALT.New'HttpCommand'('Get' url)).Run

  4. valueOf can be simplified a lot:

    valueOf←{(⊢/⍵)⊃⍨(⊣/⍵)⍳⊂⍺}

  5. I strongly recommend a strict naming convention. I'd personally name functions with initial Capital and variables with initial minuscule: ValueOf and Split

  6. Split could use instead of inverting ~ the equality =. You could also consider using a train:

    Split←≠⊆⊢

  7. There's an I-beam to get a proper temporary directory, which will allow your function to work even if there's no tmp environment variable:

    tmp_path←739⌶0

  8. I personally try to avoid parentheses, especially to govern order of execution, so I'd write the tmp_ft line as:

    tmp_ft←0 ⎕NCREATE⍠'Unique' 1⍨tmp_path,'\tmp.',fmt

  9. The final expression that computes the result is the only real bug you have. You're using ⊥⍣¯1 to automatically determine how many base-256 "digits" to use. However, if your image has no red (or no red and no green) signal at all, you will end up with only two "digits", and the format will be all wrong. E.g. if the image was entirely cyan (\$\color{#0FF}{⬤}\$ #00FFFF), all the the pixels would be encoded as 65535, which would become 255 255 instead of 255 255 255. You must therefore specify the full radix:

    r←255÷⍨bm.CBits⊤⍨3⍴256

Here is your function with all my suggested changes:

 r←GetImg url
 :With ⎕NS'url'
     ValueOf←{(⊢/⍵)⊃⍨(⊣/⍵)⍳⊂⍺}
     Split←≠⊆⊢

     tmp_path←739⌶0

     res←(⎕SE.SALT.New'HttpCommand'('Get'url)).Run

     (type fmt)←'/'Split'Content-Type'ValueOf res.Headers
     tmp_ft←0 ⎕NCREATE⍠'Unique' 1⍨tmp_path,'\tmp.',fmt
     tmp_name←⊃⎕NINFO tmp_ft
     res.Data ⎕NREPLACE tmp_ft 0
     ⎕NUNTIE tmp_ft
     'bm'⎕WC'Bitmap'('File'tmp_name)
     ⎕NDELETE tmp_name
     r←255÷⍨bm.CBits⊤⍨3⍴256
 :EndWith
\$\endgroup\$
2
  • \$\begingroup\$ Thank you. One thing I want to ask is that, for the last point (9th), I did it that way to handle images with transparency, which will have an additional alpha channel in the front. After reading your answer, I used this which seems to be working: r←255÷⍨bm.CBits⊤⍨256⍴⍨3+24≤2⍟⌈/,bm.CBits But is there a better way to do that? \$\endgroup\$
    – leo3065
    Aug 27, 2021 at 3:02
  • \$\begingroup\$ @leo3065 I'm frankly surprised that this works, and I'll take it up with the colleague that deals with this, but he's on holiday until Wednesday. \$\endgroup\$
    – Adám
    Aug 27, 2021 at 12:02

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.