1
\$\begingroup\$

I am new in JS, I am writing some extra filters on the Interface. So extra filter needs to add or update the parameter in the URL.

Option 1. No parameter in existing URL:

url = https://custom.net/

We want to add new parameter in the URL where key issector and value is IndependentConsultingDoctors

Output will be: https://custom.net/?sector=IndependentConsultingDoctors

Option 2. Update value in parameter of existing URL

URL = https://custom.net/?filter=title&sector=1234&sortby=1&page=1

We want to update parameter in the URL where key issector and value is IndependentConsultingDoctors

Output will be: URL = https://custom.net/?filter=title&sortby=1&page=1&sector=IndependentConsultingDoctors

Option 3. Delete and add parameter:

URL = https://custom.net/?filter=title&reportype=1234&sortby=1&page=1

We want to add new parameter in the URL where key issector and value is IndependentConsultingDoctors and delete reportype and inspectiontype parameters.

Output will be: URL = https://custom.net/?filter=title&sortby=1&page=1&sector=IndependentConsultingDoctors

The following is my code:

$(this).find('.btn').click(function(){  
    //var pathname = window.location.href;
    var pathname = "https://abc.com/?filter_by=title&filter_condition=&sector=1234&reportype=3455&sortby=1&page=1&itemperpage=10&input_list=&current_selected_option=2&masterfilter=%26itemperpage%3D10%26filter_by%3DAll%26filter_condition%3D%26sortby%3D1%26from_date%3D%26to_date%3D&from_date=&to_date="
    // Key and Value which need to add in paramaetrs  
    var key = "sector"
    var value = "Acutes";
    // Delete key from the parameters
    var delete_key = ["reportype", "inspectiontype"];

    // Split URL BASE and Get Parameters
    split_href = pathname.split("?");

    //No parameter present   
    if (split_href.length==1) {
         window.location.href = split_href[0]+'?'+key+'='+value;
    }else{
        // Create dictionary of parameter
        var para_dict = {};
        var para_list = split_href[1].split("&");
        for (ii in para_list){
            var tmp = para_list[ii].split("=");
            para_dict[tmp[0]] = tmp[1];
        }
        // Add target kay and Value
        para_dict[key] = value;

        // Delete key from the dinctionary
        for (ii in delete_key){
            delete para_dict[delete_key[ii]];       
        }
        //Create New Parameter string
        var new_para = "?";
        for (ii in para_dict){
            new_para = new_para+ii+"="+para_dict[ii]+"&";   
        }
        console.log(split_href[0]+new_para);
    }

});

Can you review my code and give me a more optimized solution?

\$\endgroup\$
1
\$\begingroup\$

Your code looks good, but a few typographical things I can see are:


Whitespace:

if (split_href.length==1) {
window.location.href = split_href[0]+'?'+key+'='+value;
}else{
new_para = new_para+ii+"="+para_dict[ii]+"&";
console.log(split_href[0]+new_para);

You need to include whitespace around your variables when you join them.

//Create New Parameter string
//No parameter present
//var pathname = window.location.href;   

should have whitespace between the // and the content.


In for (ii in para_list) and for (ii in delete_key), you should use i instead of ii.


I don't know whether this is controllable by you, but "reportype" is misspelt.

// Add target kay and Value
// Delete key from the dinctionary

are misspelt also.


Using para everywhere is bad practice. Use parameter instead, it's much clearer.

\$\endgroup\$
2
\$\begingroup\$

It's good to follow the single responsibility principle. The current method does many things that would make sense to extract to smaller helper functions, for example:

  • convert query string to dictionary
  • concert dictionary to query string
  • delete list of keys from dictionary
  • ... and so on

What's the point? Next time you need something similar (and most certainly you will), you will be able to reuse the common elements easier.

And yeah, as @Quill said, the names are horrible. Some ideas:

  • params instead of para_dict
  • index instead of ii
  • key_value instead of tmp
  • url instead of pathname
\$\endgroup\$
  • \$\begingroup\$ yes, Correct. I will update code tomorrow and again pass for review.Thank You. \$\endgroup\$ – Vivek Sable Jun 17 '15 at 16:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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