Trying to eliminate Redundant code
CodeGuru Home VC++ / MFC / C++ .NET / C# Visual Basic VB Forums Developer.com
Results 1 to 8 of 8

Thread: Trying to eliminate Redundant code

  1. #1
    Join Date
    Jun 2008
    Posts
    154

    Trying to eliminate Redundant code

    I got several properties that are almost exactly the same, I'm trying to merge them down to just one property, but I'm not doing it right. Here are the over-redundant properties

    Code:
            public string[] GetShieldBucklerNamesList
            {
                get
                {
                    string[] shields;
                    ArrayList ShieldsToAdd = new ArrayList();
                    foreach (KeyValuePair<string, Tuple<ShieldType, int, int>> shield in InvShields)
                    {
                        if (shield.Value.First == ShieldType.Buckler)
                        {
                            string shieldName = shield.Key;
                            ShieldsToAdd.Add(shieldName);
                        }
                    }
                    shields = ShieldsToAdd.ToArray(typeof(string)) as string[];
                    return shields;
                }
            }
    
            public string[] GetShieldHeaterNamesList
            {
                get
                {
                    string[] shields;
                    ArrayList ShieldsToAdd = new ArrayList();
                    foreach (KeyValuePair<string, Tuple<ShieldType, int, int>> shield in InvShields)
                    {
                        if (shield.Value.First == ShieldType.Heater)
                        {
                            string shieldName = shield.Key;
                            ShieldsToAdd.Add(shieldName);
                        }
                    }
                    shields = ShieldsToAdd.ToArray(typeof(string)) as string[];
                    return shields;
                }
            }
            public string[] GetShieldKiteNamesList
            {
                get
                {
                    string[] shields;
                    ArrayList ShieldsToAdd = new ArrayList();
                    foreach (KeyValuePair<string, Tuple<ShieldType, int, int>> shield in InvShields)
                    {
                        if (shield.Value.First == ShieldType.Kite)
                        {
                            string shieldName = shield.Key;
                            ShieldsToAdd.Add(shieldName);
                        }
                    }
                    shields = ShieldsToAdd.ToArray(typeof(string)) as string[];
                    return shields;
                }
            }
            public string[] GetShieldRoundNamesList
            {
                get
                {
                    string[] shields;
                    ArrayList ShieldsToAdd = new ArrayList();
                    foreach (KeyValuePair<string, Tuple<ShieldType, int, int>> shield in InvShields)
                    {
                        if (shield.Value.First == ShieldType.Round)
                        {
                            string shieldName = shield.Key;
                            ShieldsToAdd.Add(shieldName);
                        }
                    }
                    shields = ShieldsToAdd.ToArray(typeof(string)) as string[];
                    return shields;
                }
            }
    here is how I am trying to slim it down... but Im getting a ; expected error.. Am i missing something simple?? I can't see it

    Code:
            public string[] GetShieldByTypeNamesList(ShieldType shieldType)
            {
                get
                {
                    string[] shields;
                    ArrayList ShieldsToAdd = new ArrayList();
                    foreach (KeyValuePair<string, Tuple<ShieldType, int, int>> shield in InvShields)
                    {
                        if (shield.Value.First == shieldType)
                        {
                            string shieldName = shield.Key;
                            ShieldsToAdd.Add(shieldName);
                        }
                    }
                    shields = ShieldsToAdd.ToArray(typeof(string)) as string[];
                    return shields;
                }
            }

  2. #2
    Join Date
    Jun 2008
    Posts
    154

    Re: Trying to eliminate Redundant code

    I think I need a delegate..... Going to learn up on Delegates.

  3. #3
    Arjay's Avatar
    Arjay is offline Moderator / MS MVP Power Poster
    Join Date
    Aug 2004
    Posts
    11,309

    Re: Trying to eliminate Redundant code

    Delegate probably won't help here.

    Just create a helper method.

    Code:
    string[] GetShield( SheildType shieldType )
    {
      var shieldList = new <string>( );
    
      foreach ( shield in InvShields )
      {
        if ( shield.Value.First == shieldType )
        {
          shieldList.Add( shield.Key );
        }
      }
      return shieldList.ToArray( );
    }
    Then reuse it...
    Code:
    public string[] ShieldBucklerNames
    {
      get { return GetShield( ShieldType.Buckler ); }
    }
    
    public string[] ShieldHeaterNames
    {
      get { return GetShield( ShieldType.Heater ); }
    }
    ...
    TIP: Prefer to use List<> over the old fashioned ArrayList. The ArrayList class isn't typesafe which can lead to runtime errors. The List<> is typesafe so if you try to add a different type to it, you'll get a compiler error.

    Naming Comments:
    Properties don't need to include the prefix Get. (i.e GetShieldBucklerNamesList was renamed to ShieldBucklerNames)
    Local variables should be camelCase. Since class properties are PascalCase, it's confusing if you name a local variable in PascalCase (because it implies the variable isn't local).
    For example,
    Code:
    ArrayList ShieldsToAdd = new ArrayList( );
    should be
    Code:
    ArrayList shieldsToAdd = new ArrayList( );
    or (better)
    Code:
    var shieldsToAdd = new ArrayList( );
    or (best)
    Code:
    var shieldList = new List<string>( );
    Last edited by Arjay; November 18th, 2010 at 07:23 PM.

  4. #4
    Join Date
    Jun 2008
    Posts
    154

    Re: Trying to eliminate Redundant code

    Fantabulous. Had to clean up the code a little but it works like a charm. I got a question though..

    what is the difference between this

    var shieldList = new List<string>( );

    and this

    List<string> shieldList = new List<string>( );

    and why should I be using var inside this method at this time?

  5. #5
    Arjay's Avatar
    Arjay is offline Moderator / MS MVP Power Poster
    Join Date
    Aug 2004
    Posts
    11,309

    Re: Trying to eliminate Redundant code

    var just makes the code a bit more compact.

    for example,
    Code:
    Dictionary< string, MyClass > myClassDict = new Dictionary< string, MyClass >( );
    becomes
    Code:
    var myClassDict = new Dictionary< string, MyClass >( );
    less typing, easier to read, etc.

    Coming from a C++ background, I had a bit of trouble using 'var' when it first came out in C# 3.0 I thought it was like the var in VB or Javascript wherein it represents an object (like void* in C++). In others words, it's not typesafe (which can result in runtime errors).

    The var in C# is different because it's typesafe so it helps reduce code size for local variables without any of the drawbacks.

  6. #6
    Join Date
    Jun 2008
    Posts
    154

    Re: Trying to eliminate Redundant code

    yes but - you can't declare a var outside of a method, for instance it cannot be a field or property. Maybe it goes to garbage collection faster than something properly declared in a field??

  7. #7
    Arjay's Avatar
    Arjay is offline Moderator / MS MVP Power Poster
    Join Date
    Aug 2004
    Posts
    11,309

    Re: Trying to eliminate Redundant code

    You can only declare a var in a method where it's being assigned at declaration time.

    It wouldn't make sense to be able to declare a class property or field as a var because the compiler wouldn't know what type it would need to be.

    Vars are a preference kind of thing - use them or don't use them, it's your preference.

  8. #8
    Join Date
    Dec 2007
    Posts
    234

    Re: Trying to eliminate Redundant code

    var makes use of Option Infer... where at compile time the compiler will infer the type of the object based on what's being assigned to it. It can be used at any scope (as far as I am aware), but as arjay noted, it needs to be assigned a value at the same time.

    -tg
    * I don't respond to private requests for help. It's not conducive to the general learning of others.-I also subscribe to all threads I participate, so there's no need to pm when there's an update.*
    * How to get EFFECTIVE help: The Hitchhiker's Guide to Getting Help - how to remove eels from your hovercraft *
    * How to Use Parameters * Create Disconnected ADO Recordset Clones * Set your VB6 ActiveX Compatibility * Get rid of those pesky VB Line Numbers * I swear I saved my data, where'd it run off to???
    * On Error Resume Next is error ignoring, not error handling(tm). * Use Offensive Programming, not Defensive Programming.
    "There is a major problem with your code, and VB wants to tell you what it is.. but you have decided to put your fingers in your ears and shout 'I'm not listening!'" - si_the_geek on using OERN
    MVP '06-'10

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •  


Windows Mobile Development Center


Click Here to Expand Forum to Full Width

This is a CodeGuru survey question.


Featured


HTML5 Development Center