5
\$\begingroup\$

This template class is a linear search algorithm with a simple search function. I have tested it with int and char and it seems to be working fine. I would like pointers on how I can make it more efficient and on my coding technique. I do know that the standard library provides this algorithm, i am just doing it for fun and practice

sLinear.h

//Linear search template

#ifndef SLINEAR_H
#define SLINEAR_H

namespace fm
{
    template <typename T>
    class sLinear
    {
        T *_list;
        T _item;
        int _sizeOfList;

    public:
        sLinear();
        sLinear(T *myArray, T item, int size);

        ~sLinear();

        int findItem();

    };


    //------public methods------
    template <typename T>
    sLinear<T>::sLinear()
    {
        _list = nullptr;
        _sizeOfList = 0;
    }

    template <typename T>
    sLinear<T>::sLinear(T *myArray, T item, int size)
    {
        _list = myArray;
        _item = item;
        _sizeOfList = size;
    }

    template <typename T>
    sLinear<T>::~sLinear()
    {
        _list = nullptr;
        _sizeOfList = 0;
    }

    template <typename T>
    int sLinear<T>::findItem()
    {
        while (_list != nullptr)
        {
            for (int i = 0; i < _sizeOfList; i++)
            {
                if (_list[i] == _item)
                {
                    return i + 1;
                }

            }

            return -1;

        }
    }
}
#endif

main.h

#include<iostream>
#include "sLinear.h"

using namespace fm;


int main()
{

    /* Linear search with integer */
    int iList[] = { 1,9,2,6,5,3,7,4,8,0 };
    int size = (sizeof(iList) / sizeof((iList[0])));

    sLinear<int> mySearch(iList, 8, size);
    int k = mySearch.findItem();

    if (k == -1)
        std::cout << "Item not found!" << std::endl;
    else
        std::cout << "Item found:" << k << std::endl;


    return 0;
}
| improve this question | | | | |
\$\endgroup\$
4
\$\begingroup\$
  • Returning int is dubious; it unnecessarily narrows the range of possible return values. Consider retuning an iterator.

  • The line if (_list[i] == _item) implies that _list must provide an operator[](int), which is very restrictive. A linear search is just like its name implies, linear. It is expected to work on any linear collection, e.g. forward iterator (maybe even on input iterator).

  • The class sLinear exposes one public method, and keeps no state. There's no reason to make it a class. A standalone

    template <typename I>
    I findItem(I first, I last, I::value_type value) {
        ....
    }
    

    will do as well.

  • while (_list != nullptr) is very strange. _list never changes; why do you want to loop?

    PS: Could you explain a rationale for returning i+1?

| improve this answer | | | | |
\$\endgroup\$
  • \$\begingroup\$ Thank you for your help. I am returning i+1 because in arrays index starts from 0, but most people read from 1. \$\endgroup\$ – BlooB Apr 23 '17 at 5:01
  • 3
    \$\begingroup\$ @dirty_feri Programmers read from 0. Non-programmers will hardly use your code. \$\endgroup\$ – vnp Apr 23 '17 at 5:05
  • \$\begingroup\$ good point, I will implement the change. \$\endgroup\$ – BlooB Apr 23 '17 at 5:11
  • 1
    \$\begingroup\$ @vnp, it would be great to use std::iterator_traits<I>::value_type, since it will work for pointers as well. \$\endgroup\$ – Incomputable Apr 23 '17 at 8:23
  • 1
    \$\begingroup\$ Also missing typename will cause compilation error :) \$\endgroup\$ – Incomputable Apr 23 '17 at 8:30

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.