Updates from kmitov Toggle Comment Threads | Keyboard Shortcuts

  • kmitov 7:03 am on March 1, 2021 Permalink |
    Tags: , ,   

    JSON.parse(“1”) works. JSON.parse(“k1”) throws an exception 

    (Everyday Code – instead of keeping our knowledge in a README.md let’s share it with the internet)

    TL; DR; – JSON.parse(“1”) works. JSON.parse(“k1”) throws an exception

    This is a valid JSON

    "1"

    This is NOT a valid JSON

    "k1"

    The code

    $ irb
2.6.5 :001 > require 'json'
 => true 
2.6.5 :002 > JSON.parse("1")
 => 1 
2.6.5 :003 > JSON.parse("k1")
Traceback (most recent call last):
        6: from /home/kireto/.rvm/rubies/ruby-2.6.5/bin/irb:23:in `<main>'
        5: from /home/kireto/.rvm/rubies/ruby-2.6.5/bin/irb:23:in `load'
        4: from /home/kireto/.rvm/rubies/ruby-2.6.5/lib/ruby/gems/2.6.0/gems/irb-1.0.0/exe/irb:11:in `<top (required)>'
        3: from (irb):3
        2: from /home/kireto/.rvm/gems/ruby-2.6.5/gems/json-2.5.1/lib/json/common.rb:216:in `parse'
        1: from /home/kireto/.rvm/gems/ruby-2.6.5/gems/json-2.5.1/lib/json/common.rb:216:in `parse'
JSON::ParserError (809: unexpected token at 'k1')
2.6.5 :004 > JSON.parse("\"1\"")

    It is always mistakes as simple as that, that are causing the bugs.

    Details

    I am sharing the details mostly for our team.

    When users use the Instructions Steps framework on the BuildIn3D and FLLCasts platforms we have a communication between the client and the server. The client sends information as JSON, the server parses it. For a few days some of the communication was missed. Not everything was recorded on the server. This was not anything critical, but about 360,000 events were not recorded and it shows how we did not understand JSON.parse entirely.

    The commit

    The commit to cause this was 

           else
+        data = JSON.parse(events_array[0]["data"])
+        referrer = data["referrer"] if data.is_a? Hash
+

    We get a string from events_array[0][“data”] and we parse this string. But this string is not always a JSON. It is sometimes just pure string. All the specs pass and we are committing the code.

    In the specs suite all the data that we are testing is “1”,”,2″- generally numbers. Why? Because this makes sense looking at the data of the spec and in the spec we’ve had only numbers as the string, but not a general string like “k1”.

    It bites. It bites like a shark.

    FabBRIX WWF, Shark in 3D building instructions
     

  • kmitov 7:37 am on February 19, 2021 Permalink |
    Tags: , typescript   

    “In one paragraph or less” – why I chose JavaScript over TypeScript 

    My idea with this article is to try to summarize and share “in one paragraph or less” the main reason why, as a CTO, I chose one technology over another. We take everything into account, infrastructure, team, business requirements, and many others, which could be quite complex, but can we share the essence.

    In one paragraph or less

    “I chose vanilla JavaScript compiled with Google Closure Compiler and not TypeScript because I was building an extensible plugin based framework and I wanted to allow each and every plugin developer to be free to choose vanilla JavaScript or TypeScript for their plugins. I did not want to limit anyone’s future choices.”

    The pre-story (if you are interested)

    A few days ago I called an old friend. We’ve been acting like CTOs of two companies for a couple of years (I was more acting, he was doing like the real deal). But we haven’t talked in a while. The conversation went pretty fast from “How are things at work and at home?” to “Why did you choose this technology over that technology?”

    He: – We made some interesting things on the technology front.

    Me: – Really? What?

    He: – We went for the React, TypeScript path and did…

    Me: – When I had to make this decision I stayed on the Rails, Stimulus with vanilla JS path.

    He: – You know, if it wasn’t for this and that, I would have done as you did. But you should definitely check GraphQL in more detail.

    Me: – Oh, I have and …

    This conversation went on for some time.

    I had a very similar conversions about an year ago with another CTO friend that said:

    I chose “technology X” instead of “technology Y” to keep some sanity in our team.

    So we make these decisions, but can we communicate them?

     

  • kmitov 7:36 pm on February 15, 2021 Permalink |
    Tags: , , ,   

    Usage of ActiveRecord::Relation#missing/associated 

    Today I learned that Rails 6.1 adds query method associated to check for the association presence. Good to know. I share it with the team at Axlessoft along with the community as a whole.

    While reading the article about the methods I thought:

    Are we going to use these methods in our code base?

    I did some greps and it turns out we won’t.

    $ git grep -A 10 joins | grep where.not | grep nil | wc -l
0

    There is not a single place in our code that we call 

    .joins(:association).where.not(association: {id: nil}

or

.joins(:association).where(association: {id: nil})

    What does this mean?

    Nothing actually. Great methods. I was just curious to see if we will be using them in our code base a lot.

     

  • kmitov 6:39 pm on February 15, 2021 Permalink |
    Tags: ,   

    “if statements” are a “code smell” 

    (Everyday Code – instead of keeping our knowledge in a README.md let’s share it with the internet)

    Today we made a code review on a feature. Generally the review process in our team is – someone drops a message in the slack channel and says – “hey, I have these changes. Can you please take a look at them?”. We do not require a merge from an “authority” to get something to production. We also have one rule we are trying to follow:

    Your commit should make the product better than it previously was.

    https://www.axlessoft.com/careers/

    The commit below did not make the code better and this is the article about it. I am sure it would be useful for all of our team members and I hope it will be useful for the community as a whole.

    How “if statements” are a sign of “code smell”

    This is the commit. Do you see the problem with it?

       /**
    * @private
    * @param      {IS.StepsTree.LoadedEvent} event
    */
   onStepsTreeLoaded(event) {
-    this._buildId = this.generateBuildId();
+    if (!this._modeChangeOccured) {
+      this._buildId = this.generateBuildId();
+    } else {
+      this._modeChangeOccured = false;
+    }
+

    The logic in the onStepsTreeLoaded method has significantly changed. It was a simple initialization of a private variable. Now it is an if statement with an else that sets the variable used in the if to a false value.

    Wow. This is even difficult to explain.

    Why was the change introduced and how the “code smell” helped us improve?

    The thing is that our colleague had to do this change to keep the behavior of the code based on a commit from 7 months ago. But now we see this smelly code and we thought:

    Why is this even needed in the first place? Why do we call this onStepsTreeLoaded method and what is it doing for us?

    Turns out that we can just move the initialization from onStepsTreeLoaded method to another method called at a different place and we can delete this method. We would keep the same behavior. There will be no regressions. The framework has changed in the last few months to the point that there is now a better place for this initialization to happen.

    My point is: “if statement”==”code smell”

    Adding an if statement to a working code is probably a code smell. Wrapping an existing code in an if statement based on unrelated state with an else that sets this same state is probably the precise definition of what “code smell” is.

    Conclusion

    Revise your assumptions. Don’t add the if statements. Think again if this is really needed, why it is needed and where is its place in the architecture of the platform.

    The concerned emotion

    Can’t think of a better way to show you the emotion of code smell than to show you this Gorilla.

    FabBRIX WWF, Gorilla in 3D building instructions
     

  • kmitov 10:20 am on February 4, 2021 Permalink |
    Tags: , ,   

    RSpec Matchers should be simple 

    (Everyday Code – instead of keeping our knowledge in a README.md let’s share it with the internet)

    Today on a review I saw an RSpec Matcher that was more complex than it should be. We’ve discussed it internally. I think this article could be useful to our team and to the community as a whole.

    An RSpec Matcher should be simple. It should return just ‘true’ or ‘false’

    IRL example of a complex RSpec Matcher

    This is the real live example of an RSpec Matcher from todays review

      RSpec::Matchers.define :have_content_picture_type_set_to do |picture_ref, content_picture_type, content_object|
    match do |page|
      content_object_undersore = content_object.model_name.to_s.underscore
      content_object_undersore = "tutorial" if content_object_undersore == "episode"
      content_object_undersore = "courses/#{content_object.course.to_param}/section" if content_object_undersore == "course_section"

      picture_ref_row = page.find(:xpath, "//tr[.//img[@src='#{picture_ref.content_picture.image_url}']]")
      js_agnostic_click_table_action picture_ref.to_param, "/author/#{content_object_undersore}s/#{content_object.to_param}/pictures/#{picture_ref.to_param}/edit", "Edit Picture"

      page.select content_picture_type, from: "content_picture_ref[content_picture_type]"

      click_on_and_expect "Update Picture", "Picture successfully updated"
      expect(page).to have_picture_ref_with_content_picture_type picture_ref, content_picture_type
    end
    failure_message do |page|
      "expected page to have content_picture with the specified type, but was #{page.body}"
    end
  end

    There is a lot to unpack here.

    1. We do some calculations of paths with the whole content_object_underscore logic
    2. Then we find something on the page with the page.find method
    3. Then we call ‘js_agnostic_click_table_action‘ which is a complex method
    4. Then we select something from an HTML select
    5. Then we click a button with the click_on_and_expect
    6. At the end we expect something

    We should have only step 6. That’s the purpose of an RSpec Matcher. Return true or false if the page contains something or not.

    Why has this happened?

    Because RSpec.define :matcher is a function like any other ruby function. And as a ruby function you can do whatever you want with it. You can call anything you want in this function.

    Why should an RSpec Matcher be simple?

    That’s the convention for RSpec Matchers. Imagine the following spec:

        visit_pictures material
    expect(page).to have_picture_ref_with_content_picture_type material.content_picture_refs.first, "Thumbnail"

    With this spec you would expect visit a page and check if there is something on the page. But now have_pictuer_ref_with_content_picture_type makes calls, changes the db, modifies the current page and there is no way for you to understand it from reading the spec. Let alone reuse the Matcher in another situation.

    What feature is this about?

    The feature above is about showing a picture on the animated 3D models and building instructions that we show at BuildIn3D. Authors could choose which picture is the Thumbnail – example is this FabBrix Hen :).

    FabBRIX Farm Animals, Hen in 3D building instructions
     

  • kmitov 7:48 am on February 3, 2021 Permalink |
    Tags: , , python, , ,   

    Where is the redundancy? 

    (Everyday Code – instead of keeping our knowledge in a README.md let’s share it with the internet)

    I think calling a method thrее times is redundant. But then again, you have to balance. Today’s article is about a code review that at the end took like a few hours in total of different discussions and I believe it is important. This kind of things take time. Failed builds, difficult specs.

    The IRL example – Where is the redundancy? Is this DRY?

     @dataclass(frozen=True)
 class LdrawLine(abc.ABC):
+    default_x: ClassVar[float] = 23
+    default_y: ClassVar[float] = 45
+    default_z: ClassVar[float] = 0
     """
         This is the abstract class which every LdrawLine should implement.
     """
@@ -85,11 +88,19 @@ class LdrawLine(abc.ABC):
         elif line_args[1] == "STEP":
             line_param = _Step()
         elif line_args[1] == "ROTSTEP":
-            rostep_params = {
-                "rot_x": float(line_args[2]),
-                "rot_y": float(line_args[3]),
-                "rot_z": float(line_args[4])
-            }
+            if line_args[2] == "END":
+                rostep_params = {
+                    "rot_x": LdrawLine.default_x,
+                    "rot_y": LdrawLine.default_y,
+                    "rot_z": LdrawLine.default_z,
+                    "type": "ABS"
+                }
+            else:
+                rostep_params = {
+                    "rot_x": float(line_args[2]),
+                    "rot_y": float(line_args[3]),
+                    "rot_z": float(line_args[4])
+                }

    This piece of code (along with a few other changes in the commit) were the root of a 2 hours discussion in the team. A spec failed because some things were int while we were expecting them to be float.

    Calling ‘float’ three times like this is redundant

    The reason I think like this is that if you have to change and would like to have a double value or an int value you would have to change the code in three places.

    Probably e better solution would be:

    +            if line_args[2] == "END":
+                rostep_params = {
+                    "rot_x": LdrawLine.default_x,
+                    "rot_y": LdrawLine.default_y,
+                    "rot_z": LdrawLine.default_z,
+                    "type": "ABS"
+                }
+            else:
+                rostep_params = {
+                    "rot_x": line_args[2],
+                    "rot_y": line_args[3],
+                    "rot_z": line_args[4]
+                }
                  # We are adding a loop to call the float
+                for key in ["rot_x", "rot_y", "rot_z"]:
+                     rotstep_params[key] = float(rotstep_params[key])

    We call float only once at a single place. Now we have to deal with the fact that we have “rot_x” as a variable in two places, yes, that is true, but this could easily be extracted and we can iterate over the rostep_params values. But now we have consistency.

    Is it harder to read? Probably it is a little harder. Instead of simple statements you now have a loop. So you lose something, but you gain something. A float function that is called at a single place.

    What are we doing with these rotsteps?

    ROTSTEP is a command in the LDR format. We support LDR for 3D building instructions. Here is one example with a FabBrix Monster that uses the LDR ROTSTEP as a command:

    FabBRIX Monsters, Cthulhu in 3D building instructions
     

  • kmitov 7:31 am on February 3, 2021 Permalink |
    Tags: ,   

    Rails after_action order is in reverse 

    (Everyday Code – instead of keeping our knowledge in a README.md let’s share it with the internet)

    We’ve used Rails after_action for controllers twice in our platforms and products. It allows you to execute something after a rails controller action.

    Why? What is the IRL use case?

    Users upload files for a 3D model or building instructions. We want to call an external service to convert this 3D model or buildin3d instructions and we schedule a job. We want to ping this external service. So we make a network request in the controller.

    This is never a good idea. Generally you want to return fast from the call to your controller action. On Heroku you have a 15 seconds time limit to return and as a user experience it is good to return a result right the way and to do your job on the background and to report on the progress with some clever JS.

    But for this specific case we have reasons to try to ping the external service. It generally takes about 0.5-1s, it is not a bottleneck for the user experience, and it works well. Except for the cases when it does not work well.

    Today was such a day. Heroku dynos were trying to connect with the service, but it was taking longer than 15 seconds. I don’t know why. So i decided to revise the implementation.

    What is special about after_action order?

    It is the reverse of before_action.

    We want to call the external service in an after action. In this way it is outside of the logic of the controller method and if it fails, we handle the exception in the after_action. It is a good separation. The real code is:

    module IsBackend::JenkinsWorkoff extend ActiveSupport::Concern
  included do
    after_action do
      unless @jenkins_rebuild == nil || @jenkins_rebuild.running?
        begin
          JenkinsClientFactory.force_jobs_workoff timeout: 10
        rescue => e
          config.logger.info e.message
        end
      end
    end
  end
end

    The issue is with the order of the after_action when they are more then one.

    Generally before_action are executed in the order they are defined.

      before_action do
    # will be first
  end

  before_action except: [:index, :destroy] do
    # will be second
  end

    But after_action are executed in the reverse order

      after_action do
    # will be SECOND
  end

  after_action except: [:index, :destroy] do
    # will be FIRST
  end

    Why? I will leave the PR to explain it best – https://github.com/rails/rails/issues/5464

    How do we use it?

    When building the buildin3d instruction below we’ve pinged the external service in the build process. Because of the after_action we can enjoy this FabBrix dog.

    FabBRIX Pets, Dog in 3D building instructions
     

  • kmitov 7:06 am on February 3, 2021 Permalink |
    Tags: , ,   

    $ heroku whoami? 

    (Everyday Code – instead of keeping our knowledge in a README.md let’s share it with the internet)

    Today many things stopped in our dev procedures and I had to learn about heroku auth:whoami and to use it. Luckily, nothing about production, but a few engineers were puzzled as to why certain dev services are not working.

    Heroku CLI logged itself out

    We are using Heroku CLI a lot for a lot of different things. One example is backups. We backup the Heroku DBs regularly – like Daily, Weekly, Monthly and some of the backups stopped working. A few other services also stopped working. The problem was with Heroku CLI. The direct log from our Jenkins was:

    heroku: Press any key to open up the browser to login or q to exit:  ▸    Invalid credentials provided.
Enter your Heroku credentials:

    Strange. It was working for years and now suddenly heroku cli decides to log itself out and it then requires us to log in. How come, we don’t know. I’ve created a Ticket with Heroku. My greatest concern is that it could be a security issue with someone gaining access to a dev machine and logging out, but I have no evidence of this.

    $ heroku auth:whoami

    I had to log in again with heroku. But I also wanted the builds that were ran with Jenkins to fail if heroku cli is not logged in. I found the command:

    $ heroku auth:whoami

    Which does the job. If we are not logged in the command will return and the bash script will fail.

    We are happy again

    This instruction is delivered with heroku 🙂

    FabBRIX Pets, Hamster in 3D building instructions
     

  • kmitov 8:14 pm on January 31, 2021 Permalink |
    Tags:   

    Ruby String#sub! and String#gsub! 

    (Everyday Code – instead of keeping our knowledge in a README.md let’s share it with the internet)

    In Ruby String#sub! is different from String#gsub!. That much we know. But I am so used to .gsub that could not even see that the code in question uses .sub

    This one got me today. In the code we had 

    self.text_content.sub!(/\( %7B%7B(%20)*/, "( {{ ")

    I went over to change the regex because of a new requirement and I deployed and published on production.

    But String#sub! replaces only the first occurrence of the regex, not all the occurrences. I could not event see that it was #sub! used and not #gsub!. Why would somebody one to replace only the first occurrence for this case – I don’t know.

     

  • kmitov 8:31 pm on January 28, 2021 Permalink |
    Tags: , , ,   

    How the software becomes unmaintainable? – a practical example 

    (Everyday Code – instead of keeping our knowledge in a README.md let’s share it with the internet)

    An ugly, sometimes over neglected truth of the industry is that the software that we work on does not become unmaintainable and tedious and difficult to work with overnight. It becomes like this with every change we introduce. While each individual change might not be “that bad” when they pile up we no longer have a well decoupled, maintainable system. We end up with a mess, which we than re-write with a new framework, a new team, new concepts and architecture just trying to do better. But the problem most of the time is in the single change that we do today – is it making the software system better or worse?

    This article is about a practical example of today and how we got to stop it.

    To better or worse

    When developing software incrementally we introduce changes. There are basically two options – the new changes would make the system better or they will make it worse.

    Here is the change from today

    --- a/file1.rb
+++ b/file1.rb
@@ -8,6 +8,7 @@ module IsBackend
 
     def embed
+      @full_video_src = @material.video_refs.where(usage_type: "full_video").first.try(:video).try(:source_url)
 
diff --git a/file2.rb b/file2.rb
index bce7cd1a0..43b3768ea 100644
--- a/file2.rb
+++ b/file2.rb
 
   before_action do
     @namespaces = [:author]
+    @full_video_src = @material.video_refs.where(usage_type: "full_video").first.try(:video).try(:source_url)
   end

diff --git a/file3.rb b/file3.rb
index 650456cb0..8c772288a 100644
--- a/file3.rb
+++ b/file3.rb
@@ -41,6 +41,8 @@ class MaterialsController < CommonController
     @client_import_path = "shared" if Rails.application.config.platform.id == "b3"
+    @preview_video_src = @material.video_refs.where(usage_type: "preview").first.try(:video).try(:source_url)
+    @full_video_src = @material.video_refs.where(usage_type: "full_video").first.try(:video).try(:source_url)
   end

    The logic is not important. We basically get the video for a material. What’s important is the code is practically the same in all 3 places. 4 calls in three files and they are all the same.

    In the past I was leading a class in Software Development. One thing I tried to teach each student was:

    When you copy and paste you introduce a bug. That’s a fact of the industry.

    The reason is that once you copy/paste you would have to support the same logic in more than one place and you would simply forget about the second place the next time you would like to change the logic. It might not be you, it might be colleagues working years from now on the same code, but they will forget to change both places. That’s the problem with redundancy.

    Remove the redundancy, and you remove most of the bugs

    How did we stop it?

    Simple review. Just ask a second person to check your commit. This simple review process protects you from a large portion of the other bugs that are still left after you’ve cleared all the redundancies (of course).

    Enjoy

    The code in question is part of the logic delivering these instructions. See how the video is displayed. That’s what the feature was about. Enjoy.

    Torvi – Ball shooting Lego machine
     

← Older posts

c
compose new post
j
next post/next comment
k
previous post/previous comment
r
reply
e
edit
o
show/hide comments
t
go to top
l
go to login
h
show/hide help
shift + esc
cancel