1

I am currently working on a program to traverse through a list of numbers with two different functions to find the sum and a specific value. Here is the code that I have implemented 

class Program
{
    static int i, sum;
    static List<int> store = new List<int>();

    static void Main(string[] args)
    {


        for (i = 0; i < 100; i++)
        {
            store.Add(i);
        }

        i = 0;
        TraverseList();
        Console.ReadLine();
    }

    static void TraverseList()
    {
        while (i < store.Count)
        {
            FindValue();
            FindSum();
            i++;
        }

        Console.WriteLine("The sum is {0}", sum);
    }

    static void FindValue()
    {           
        if (store[i] == 40)
        {
            Console.WriteLine("Value is 40");
        }                
    }

    static void FindSum()
    {
        sum = sum + store[i];
    }
}

I was thinking of separating FindSum and FindValue into two different functions and not calling them in TraverseList. Is there any other way of doing it rather the duplicating the common code of list traversal in those two functions as I have done here

class Program
{
    static int i, sum;
    static List<int> store = new List<int>();

    static void Main(string[] args)
    {


        for (i = 0; i < 100; i++)
        {
            store.Add(i);
        }

        i = 0;
        FindValue();
        i = 0;
        FindSum();

        Console.ReadLine();
    }

    static void FindValue()
    {
        while (i < store.Count)
        {
            if (store[i] == 40)
            {
                Console.WriteLine("Value is 40");
            }
            i++;
        }
    }

    static void FindSum()
    {
        while (i < store.Count)
        {
            sum = sum + store[i];
            i++;
        }

        Console.WriteLine("The sum is {0}", sum);
    }
}
Improve this question
4
  • 2
    This code is extremely bad, the most blatant offense being that your methods FindValue and FindSum use hidden parameters (store and i) for no reason. There is absolutely no need to do things this way. Throw this code away and start again. Commented Oct 6, 2013 at 19:37
  • 2
    @jon I am a beginner and I have been trying to get things working and I don't even know what hidden parameters are. I will surely do a bit of research on that. Commented Oct 6, 2013 at 19:40
  • @SriramSakthivel I haven't used linq before and dont have much about it Commented Oct 6, 2013 at 19:42
  • 1
    @AjitPeter: I am just trying to say that you have taken the wrong path here. "Hidden parameters" means that your functions need some things to work with, but that's not mirrored by their signatures (they look like they need no arguments at all). Don't ever do that. In this case, both should accept one argument and you should pass store[i] as a parameter. Commented Oct 6, 2013 at 19:42

3 Answers 3

Reset to default
1

To find the sum of a series of numbers you can use the simple LINQ function:

List<int> numbers = new List<int>();
int sum = numbers.Sum();

I am not sure what you mean by find a value. If you want to check if one of the numbers in a series is equal to a certain value you can use the LINQ function Any:

int myValue = 40;
bool hasMyValue = numbers.Any(i => i == myValue);

This uses a lambda expression which executes a function and passes each element in the collection to the function. The function returns true or false to indicate that the element is a match for the Any test.

If instead you want to check for how many numbers in a sequence match a certain value you can instead use the Count function like so:

int numberOfMatches = numbers.Count(i => i == myValue);
Improve this answer
Sign up to request clarification or add additional context in comments.

8 Comments

Don't call lambda expression parameter i - I think that's a bad practice. The OP doesn't want Any item.
FindSum() is only an example of the functionality that I am looking to achieve. So the sum() wouldn't help me. My concern is that if I were to add more functions to manipulate the list I would end up duplicating the list traversal for each function similar to FindSum() or FindValue().
@AjitPeter You would have to be more specific about exactly what you are looking to achieve for advice on how to achieve it. There are a limitless amount of things you could do with a list and a large number of ways to solve each. It sounds like you're worried about duplicating the iteration code. This is literally only one line of code if you use a simple for or foreach loop. The only way you can condense it further is to use syntactic sugar like the LINQ statements I've shown you. Chances are you can use it to solve 95% of your problems.
Please refer to my chat with @chris where I have mentioned what I am trying to achieve and why I am trying to separate the functions which use the common traverse()
It doesn't sound like you have a real world problem. You want to have a large number of different kinds of operations you could perform on a list and you want an easy way to pick and choose which operations to execute. It sounds like a ridiculous scenario where you might have 30 functions and you want to decide "only run these 23 functions". Stackoverflow isn't really for questions about what if you wanted to do something silly. See the help page. Unless you can provide an actual real world problem you need solving we aren't much help to you.
|
0

First thing - I would use foreach instead of while, regarding the duplicate code (assuming you are not using Linq) - I think it's fine

A taste how Linq can simplify your code:

var FindSum = store.Sum();
var FindValue = store.FindAll(x => x == 40);
Improve this answer

5 Comments

FindAll? use Find or Exist or Any(Linq) methods
@SriramSakthivel - No. It seems like he wants all items, look at his code, he doesn't stop iterating after finding first item.
I guess the linq implementation above will help me only this particular case but my concern is of duplicating the same code (list traversal) for any further functions that I add in the future.
@AjitPeter - YES this specific code will help you now, but if you learn some more about Linq, you can solve much more complicated problems, and then, the so called duplicated code will be in the Linq code and not yours.
Linq is really helpful with predefined functions like sum() available already
0

I cannot stress enough how bad it is to have i and sum as class members. Especially i. It will make your code very fragile, and hard to work with. Try to make each method as isolated from the rest of the code as possible.

Try something like this instead:

static void Main( string[] args )
{
    List<int> store = new List<int>();

    for( int i = 0; i < 100; i++ )
        store.Add( i );

    FindValue( store );
    FindSum( store );

    Console.ReadLine();
}

static void FindValue( List<int> list )
{
    for( int i = 0; i < list.Count; i++ )
    {
        if( list[i] == 40 )
            Console.WriteLine( "Value is 40" );
    }
}

static void FindSum( List<int> list )
{
    int sum = 0;
    for( int i = 0; i < list.Count; i++ )
        sum += list[i];

    Console.WriteLine( "The sum is {0}", sum );
}

It is perfectly fine (and normal) to duplicate the looping, it's just a single line. You could also use a foreach here.

Also, disregard everyone telling you to use LINQ. You're obviously new to programming and you should learn the basics first.

Improve this answer

11 Comments

I agree that the code here is bad even for the standards an example :) The above example is just an overview of the functionality that I am trying to achieve. The traversal list is not just a single line of code. I have around 50 lines of code and 3 functions which all use the traversal function. I thought there might be a better way of doing it rather copying the same 50 lines of code 3 times for each function.
In that case, have one loop, and then pass the list + the index to your different methods. I just wanted to stress that even for an example, never ever reuse i as you have done, it's bad for you!
Is any other way of doing it of which I am unaware of since I am a beginner. I was of the notion that code duplication is not good. Some of the comments earlier said the same that it is alright to have duplicate code.
Yeah they're right - if you find yourself rewriting the same lines of code all over the place, those lines of code should probably be moved into a method. But don't take it all too far. A single line, something as common as a for/foreach loop, is nothing to worry about.
Don't think the passing the index would work as I have to traverse through the entire list always for all the three methods.
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.