2
\$\begingroup\$

I'm building an app in angular 4. Have a listing page which shows all ads. I want to reuse same HTML/ JS for this page for all my needs. My needs are:

  1. All ads should be shown in this page;
  2. Those ads can be filtered by city/ category or both;
  3. User can also search ads;
  4. Search results can be filtered by city/ category or both.

I want to reflect all searches and filters within the url so if you post the url or enter the url in the browser I want to pull the ads based on that url.

Url example:

listing/hello-world-search-query/galway/car-services

So I came up with this if / else statement which handles this ok.

I know if else is relatively slow, compared to switch.

Here is what I came up with:

// listing page no search and no filters url: listing
if(!this.routeParameters.search) {
    this.metadataService.setTitle('Ad Listing | Stiqqs');
    this.metadataService.setTag('description', 'Listing page description goes here');
    this.metadataService.setTag('keywords', 'keywords');
    this.getNewContentAdList('get', 'all', 10000);
    console.log('No parameters');

// listing based on search url: listing/hello-world-search-query
} else if (this.routeParameters.search !== 'search' && !this.routeParameters.city && !this.routeParameters.category) {
    this.metadataService.setTitle('Ad Listing ' + this.unslugify(this.routeParameters.search) + ' | Stiqqs');
    this.metadataService.setTag('description', 'Listing page description goes here');
    this.metadataService.setTag('keywords', 'keywords');
    this.getNewContentAdList('get', 'all', 10000);
    console.log('Search parameter');

// listing based on city filter only url: listing/search/galway
} else if (this.routeParameters.search === 'search' && this.routeParameters.city !== 'category') {
    this.metadataService.setTitle('Ad Listing | Stiqqs');
    this.metadataService.setTag('description', 'Listing page description goes here');
    this.metadataService.setTag('keywords', 'keywords');
    this.getNewContentAdList('get', 'all', 10000);
    console.log('No search, but city filter applied');
}

// listing based on category filter only url: listing/search/category/car-services
else if (this.routeParameters.search === 'search' && this.routeParameters.city === 'category' && this.routeParameters.category) {
    this.metadataService.setTitle('Ad Listing | Stiqqs');
    this.metadataService.setTag('description', 'Listing page description goes here');
    this.metadataService.setTag('keywords', 'keywords');
    this.getNewContentAdList('get', 'all', 10000);
    console.log('No search, no city but category filter applied');

// listing based on search with city filter url: listing/hello-world-search-query/galway
} else if (this.routeParameters.search !== 'search' && this.routeParameters.city !== 'category' && !this.routeParameters.category) {
    this.metadataService.setTitle('Ad Listing | Stiqqs');
    this.metadataService.setTag('description', 'Listing page description goes here');
    this.metadataService.setTag('keywords', 'keywords');
    this.getNewContentAdList('get', 'all', 10000);
    console.log('Search filtered by city');

// listing based on search with category filter url: listing/hello-world-search-query/category/car-services   
} else if (this.routeParameters.search !== 'search' && this.routeParameters.city === 'category' && this.routeParameters.category) {
    this.metadataService.setTitle('Ad Listing | Stiqqs');
    this.metadataService.setTag('description', 'Listing page description goes here');
    this.metadataService.setTag('keywords', 'keywords');
    this.getNewContentAdList('get', 'all', 10000);
    console.log('Search filtered by category');

// listing based on search with city and category filter. url: listing/hello-world-search-query/galway/car-services    
} else if (this.routeParameters.search !== 'search' && this.routeParameters.city !== 'category' && this.routeParameters.category) {
    this.metadataService.setTitle('Ad Listing | Stiqqs');
    this.metadataService.setTag('description', 'Listing page description goes here');
    this.metadataService.setTag('keywords', 'keywords');
    this.getNewContentAdList('get', 'all', 10000);
    console.log('Search filtered by category and city');

// listing did not match any of the above
} else {
    console.warn('Listing did not match any queries');
}

This is not ideal and can quickly become very messy. So what I want to know if anyone could think of a better solution to this that could improve the performance?

\$\endgroup\$
  • \$\begingroup\$ Currently each branch of your code does exactly the same thing (except for your output), could it be you didn't include the real code? Also, wouldn't else mean that you should show all the ads, a bit like your first branch? \$\endgroup\$ – Icepickle Aug 2 '17 at 10:43
  • \$\begingroup\$ The code performs exactly as it is expected. Based on url it returns correct instances if that makes sense. I guess my question is could anyone suggest an if else alternative for this kinda problem maybe incorporate Switch statement somehow \$\endgroup\$ – TSlegaitis Aug 2 '17 at 10:53
  • 1
    \$\begingroup\$ I'm sorry but then where are you adding your filters? I only see a change where you are using unslugify in the second branch. For the rest each of the branches do the exact same thing... \$\endgroup\$ – Icepickle Aug 2 '17 at 10:58
  • 3
    \$\begingroup\$ I know if else is relatively slow, compared to switch not generally, no. Anyway, the difference would be nanoseconds so it's not worth even mentioning. \$\endgroup\$ – wOxxOm Aug 2 '17 at 11:05
  • \$\begingroup\$ Dont mind whats inside the if else. What I want to do is see if I can replace if else with a better performing substitute. \$\endgroup\$ – TSlegaitis Aug 3 '17 at 10:03
1
\$\begingroup\$

From the code snippet shown, I would say if the amount of data (ads) in total is not very large. The service code should simply : get all data. In another word, make HTTP request.

The filtering logic should be in another typescript/Javascript. One function should take the column/field it tries to filter as a parameter and return data set.

The advantage of Angular or AngularJS is that the UI can react very fast to user input. Maybe you want to have 1 dropdown for selecting cities, 1 dropdown for selecting categories. And then a input field for search. And these logic wouldn't be in the service code. It will be in a component.

In the scenario where data set is very large, the filtering for cities or categories can happen in the middle tier.

The logic you have currently, you are repeating a lot. For example, multiple if statement has this.routeParameters.search === 'search'

Instead of checking: if(ABC), if(A!BC), if(AB!C), if(A!B!C), try if(A){ check for b and c here }

Things to look into: Angular 2 filter Example https://plnkr.co/edit/lALGzG?p=preview

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

Regarding performance

There are three rules about performance: measure, measure, measure.

If you seek for a more performant solution, you should first determine how slow your solution currently is. So, you should measure how many seconds, milliseconds or microseconds this code takes to execute.

After that you can come asking for performance help. Telling us how slow exactly your code currently runs, why that speed is not acceptable for you, and what would be an acceptable speed.

\$\endgroup\$

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.