-1

I'm in an Intro to C++ class, and for one of our assignments we're making an online shop. One of our problems is to make a search function for the inventory, using a linear search, then display the corresponding price, stock, and shipability for that item.

For some reason, no matter how much I try and tweak it, it always returns false for the search, and that the store doesn't carry the item, even if I type in an item that I know is in the items array.

For example, if I type Moon Pie (which is in my array) in the getline, it'll still return as -1 like it isn't. Anything noticeably wrong with this code?

Here's my inputInventory.txt

Moon Pie    3.50    15  1
Cosmic Brownie  2.00    12  0
Moon Shine  7.00    7   1
Astronaut Icecream  4.00    11  1
Neptune Nuggets 2.50    30  1
Venus Vodka 6.50    10  1
Planet Pop  4.50    20  0
Starry Salad    3.00    15  0
Celeste Cakes   5.00    11  1
Plasma Potion   9.99    4   1
Star Fruit  2.50    10  1
Sun-dae 7.00    20  0
Moon Cheese 5.00    10  1
Milky Way Milkshake 6.50    5   0
Pluto Pie   7.00    9   10
#include <iostream>
#include <iomanip>
#include <fstream>
#include <string>
using namespace std;

const int MAX = 15;
void searchInventory(string itemNames[], double itemCost[], int itemNoShip[MAX][2]);
int linearSearch(string arr[], int size, string value);

int main() {
  int input;
  string items[MAX];
  double priceItems[MAX];
  int noItems[MAX][2];

  cout << "\n1. Read in Inventory\n";
  cout << "2. Display Inventory\n";
  cin >> input;

  while (input > 2 || input < 1) {
    cout << "An error has occured. Please input a value 1 - 2. >> ";
    cin >> input;
  }

  switch (input) {
    case 1:
      if (readInventory(items, priceItems, noItems) == true) {
        cout << "\nReading the file...\n";
      }
      break;
    case 2:
      searchInventory(items, priceItems, noItems);
      break;
  }
}

bool readInventory(string itemNames[], double itemCost[], int itemNoShip[MAX][2]) {
  bool fileRead = false;
  ifstream inputFile; // Pointer
  inputFile.open("inputInventory.txt");

  if (inputFile) // Test if file opened
  {
    for (int row = 0; row < MAX; row++) {
      getline(inputFile, itemNames[row], '\t');
      inputFile >> itemCost[row];
      inputFile >> itemNoShip[row][0];
      inputFile >> itemNoShip[row][1];
    }

    fileRead = true;
    inputFile.close();

  }
  return fileRead;
}

void searchInventory(string itemNames[], double itemCost[], int itemNoShip[MAX][2]) {
  string search;
  int result;
  int position;
  cout << "Please type the name of the item you are looking for. > ";
  cin.ignore();
  getline(cin,search);

  result = linearSearch(itemNames, MAX, search);

  cout << result;

  if (result >= 0) {
    cout << "\nYour item was found!\n";
    cout << itemNames[result] << itemCost[result] << itemNoShip[result][0] << "Shippable:" << itemNoShip[result][1];
  }
  else {
    cout << "\nThis item was not found in the list.";
  }
}

int linearSearch(string arr[], int size, string value) {
  int position;
  int index;
 
  for (index = 0; index < size; index++) {
    if (arr[index] == value) {
      position = index;
    } 
    else {
      position = -1;
    }
  }      
  
  return position;
}
Remy Lebeau
  • 454,445
  • 28
  • 366
  • 620
Meichi
  • 1
  • 2
  • `linearSearch()` LGTM, can you please share a Complete, Minimal and Reproducible example? For example, how is your array populated? Is `value` storing the expected value before calling the search method? Are you sure `position` gets initialized inside the search method? – gsamaras Nov 24 '20 at 23:11
  • 1
    Adding on to gsamaras, we need to know what you're passing to this function. – M-Chen-3 Nov 24 '20 at 23:11
  • `linearSearch` returns an uninitialized value if that `for` loop isn't executed at least once. What if `size` is 0? – PaulMcKenzie Nov 24 '20 at 23:12
  • 1
    Please provide a [mcve]. – Fred Larson Nov 24 '20 at 23:16
  • In addition to that, all we see are variables. We do not see the actual values being used. as they are inputted from `cin`. What if you are not typing in the values you claim you are using? You should hardcode the values instead of using `cin` to verify that the `linearSearch` function actually works. Or you could simply output: `cout << "\nThis item " << search << " was not found in the list.";`, so that you see what you are actually searching for. – PaulMcKenzie Nov 24 '20 at 23:17
  • Hey all, updated the post a bit, let me know if you guys need anything else. I'm a bit new to this community. – Meichi Nov 24 '20 at 23:47
  • @Meichi This code is incomplete, missing several vital language markups. It shouldn't even compile as shown. Please fix that. – Remy Lebeau Nov 25 '20 at 00:23
  • @RemyLebeau I think I've fixed it. Please let me know. – Meichi Nov 25 '20 at 02:03
  • @Meichi still had a few syntax mistakes. I have corrected them for you. – Remy Lebeau Nov 25 '20 at 02:19

3 Answers3

2
for (index = 0; index < size; index++) 
        if (arr[index] == value) {
          position = index;
        } 
        else {
          position = -1;
        }
      

This loop continually overwrites position.

Unless your sought-after element is the last one in the array, immediately after it's been found the next element will cause position to be set to -1 again (unless that one matches too ).

You should stop looping (or, at least, stop updating position) as soon as a match is found.

Also, it would be advisable to wrap the entire loop body in {} braces, as that is conventional and what people will expect to see, making the code easier to read and to understand.

How about:

int linearSearch(string arr[], int size, string value)
{
    for (int index = 0; index < size; index++)
    {
        if (arr[index] == value)
            return index;
    }
    
    return -1;
}
Asteroids With Wings
  • 16,164
  • 2
  • 17
  • 33
1

You should break out of the for loop once you found the item, if not the loop will just continue and overwrite position. Edited: Based on PaulMcKenzie's comment, you should initialize position with a value so that it will not return garbage value.

int linearSearch(string arr[], int size, string value) {
  int position = -1;
  int index;
 
  for (index = 0; index < size; index++) {
        if (arr[index] == value) {
          position = index;
          break;
        }
  }
      
  return position;
}
0

The problem is cin.ignore(), since the default value of the first parameter is one, the first letter will always be taken out. So if someone typed "Moon Pie", the value in search will be "oon Pie".

  • I also assume that you are `using namespace std`, otherwise "cin" and "cout" are errors as well. – UmbraSicaro Nov 24 '20 at 23:18
  • They're probably ignoring the newline from a previous, unseen, formatted extraction. – Asteroids With Wings Nov 24 '20 at 23:21
  • Hey, thank you! For some reason when I just use the getline by itself it doesnt let me input anything. I need to use the ignore function to actually input a line. – Meichi Nov 24 '20 at 23:46
  • I would suggest that you just use `std::cin >> search` then. That will allow you to input and store said input in the `search` variable. – UmbraSicaro Nov 24 '20 at 23:49
  • @UmbraSicaro Only if it's one word. Generally, `getline` is better. – Asteroids With Wings Nov 25 '20 at 00:25
  • @Meichi "*For some reason when I just use the getline by itself it doesnt let me input anything*" - then you are using it wrong, probably because of [this issue](https://stackoverflow.com/questions/21567291/). @UmbraSicaro "*I would suggest that you just use `std::cin >> search`*" - that won't allow searching for items with spaces in their names. `std::getline()` is more appropriate (when used properly) – Remy Lebeau Nov 25 '20 at 00:26
  • @Remy I don't think it's fair to say they're "using it wrong", when they're correctly using the workaround for formatted extraction followed by `getline`, and when that code is literally what we're talking about... – Asteroids With Wings Nov 25 '20 at 00:27