0

So I wrote some code and was told it was poor form to write it in this way.

So hoping someone can point me towards a better way. My code:

string phrase = Value.Text;
List<string> wordList = new List<string>();
string words = phrase.Split('\t', '\r');

int Three = 3;
int Two = 2;
int One = 1;
int Four = 4;
int Five = 5;
int Six = 6;

for (int i = 0; i < result; i = i + 12, Three = Three + 12, 
Two = Two + 12, One = One + 12, Four = Four + 12, Five = Five + 12, 
Six = Six + 12)
{
wordList.Add(ID.Text + '\t');
wordList.Add(IT.Text + '\t');
wordList.Add(words[Three] + '\t');
wordList.Add(words[Two] + '\t');
wordList.Add(PD.Text + '\t');
....etc.
}

The offending part is this:

for (int i = 0; i < result; i = i + 12, Three = Three + 12, 
    Two = Two + 12, One = One + 12, Four = Four + 12, Five = Five + 12, 
    Six = Six + 12)

I can see why, it is pretty ugly and quite long, perhaps unnecessarily so?

What are you trying to do?

I have an array of data (phrase) that is entirely random.

The data is entered in sets of 12, but more than one data set can be entered, but always divisible by 12 - so 12, 24, 36. I do not know how much data will be entered at any given point, but I know it will be divisible by 12.

Sometimes words, sometimes numbers - I do not know in advance what it will be, they are not days, they are not ages, I cannot specify an element to the data points in the array. They will be varying in character length, they will have different punctuation marks in different places in each data set. All of the data is separated by a Tab.

From the list of 12, I need to pull, the 3, 2, 1, 4, 5, 6.

In that order. I then place that data into a List<> and output the data, along with some text boxes to match a specified format.

Purpose of the code:

To loop through the data in sets of 12, until there is no more data.

Each loops pulls a data point from the array, for one example, the 3rd, 15th, 27th, 39th etc.

Each loop adds the data from the words array, adds TextBoxes in various places and then compiles them in a List. The List is then produced for the user to copy into a specified format in a single action.

I thought up the "int Three = 3;" for the purpose of being able to increase that number by 12 each loop. I couldn't think of any other way to do it.

But here to learn, so if you know how it should be done - I am all ears, even if you just want to point me to what I should be doing.

If you need an example of the data, face palm your keyboard 12 times putting a tab between each one...you will have an idea of what the data is - there is absolutely no point me providing an example, it is meaningless gibberish, the only factor that is consistent is the Tab between each grouping.

  • If all you need is the per index offset you can calculate it directly from the current value of `i`. Off the top of my head I'd create an object that represents 1 distinct group of data, then loop the `words` collection with a 12 item inner loop to populate it, then use that to build the final structure. – asawyer Aug 28 '20 at 01:50
  • So I could use i + 4 to specify the value for the first 'words[3]', but wouldn't I end up with a similar piece of code? int AB = i + 4, words[AB], and then wouldn't I have to put that into my loop for the changes to i to effect AB? I looked at objects before, I could not find a way to get the words[] into an object and then change the words[] within the object. Without spacing and // for context...this is 31 lines of code to complete this task. –  Aug 28 '20 at 03:14
  • Having slept on it, this would fix my lengthy FOR code, even if the i + 4 went into the loop. So I did it, and it freaked me out because x = i + 4 didn't work, but x = i + 3 did work. i = 0 at the start, so figured it would require 4 to become 3, but it didn't. Gonna have to think on that for awhile. –  Aug 28 '20 at 08:51
  • having a _variable_ called `Three` whose value will be 15 or something else while running, well... that's offending!. Correctly naming variables and method is important, you or someone else will have to read you code sooner or later, make your like easier using meaningful names – Gian Paolo Aug 28 '20 at 14:19
  • I named it three, so I knew where to put it in the output list...three went into the first word, two to the second, one to third and so on to match the 3, 2, 1, 4, 5, 6 that I needed....made sense when I did it ! lol –  Aug 28 '20 at 20:46

1 Answers1

0

To long for a comment - Here's a prospective solution implementing what I outlined earlier. It breaks the input into 12 length chunks and creates a temporary collection of word sets that could be used for whatever purpose you need with comments explaining how it works.

void Main()
{
    int demoDataSetCount = 10;
    int chunkSize=12;
    //create some test data as a multiple of the chunk size
    var inputData = string.Join(Environment.NewLine, Enumerable.Range(0, demoDataSetCount * chunkSize)
        .Select(e => string.Join("\t", Enumerable.Range(e, chunkSize))));
    
    //call the extension method
    var wordSets = inputData.GenerateWordSets(chunkSize);
    
    //loop over the resulting collection
    foreach(var wordSet in wordSets)
    {
        //get the various interesting items by index
        var three=wordSet.Values.ElementAt(2);
        var two=wordSet.Values.ElementAt(1);
        var one=wordSet.Values.ElementAt(0);
        var four=wordSet.Values.ElementAt(3);
        var five=wordSet.Values.ElementAt(4);
        var six=wordSet.Values.ElementAt(5);

        //write to console demo'ing output
        Console.WriteLine($"Index: {wordSet.Index}{Environment.NewLine}\tThree: {three}{Environment.NewLine}\tTwo: {two}{Environment.NewLine}\tOne: {one}{Environment.NewLine}\tFour: {four}{Environment.NewLine}\tFive: {five}{Environment.NewLine}\tSix: {six}");
    }
}

//used to hold the intermediate output, one set of words per chunk size
public sealed class WordSet
{
    public int Index { get; set; }
    public IEnumerable<string> Values { get; set; }
}

public static class Extensions
{
    //various optional parameters to tweak behavior
    public static IEnumerable<WordSet> GenerateWordSets(this string input, int chunkSize=12, int interestingRangeStart=0, int interestingRangeLength=6)
    {
        //split the input on new line and tabs
        var words = input.Split(new string[] { Environment.NewLine, "\t" }, StringSplitOptions.None);
        //generate a range of ints to group with
        return Enumerable.Range(0, words.Length)
            //int rounding used to group into chunks
            //get the words from the current index
            .GroupBy(k => k / chunkSize, k=> words[k])
            //project grouping into a WordSet collection
            .Select(g => new WordSet 
            { 
                Index = g.Key,
                //take only the indexes we are interested in reading
                Values = g.Skip(interestingRangeStart).Take(interestingRangeLength) 
            });
    }
}

See https://stackoverflow.com/a/1008974/426894 for more on how the grouping works here.

Input like this:

0 1 2 3 4 5 6 7 8 9 10 11
1 2 3 4 5 6 7 8 9 10 11 12
2 3 4 5 6 7 8 9 10 11 12 13
....

Generates Output like:

Index: 0
  Three: 2
  Two: 1
  One: 0
  Four: 3
  Five: 4
  Six: 5
Index: 1
  Three: 3
  Two: 2
  One: 1
  Four: 4
  Five: 5
  Six: 6
Index: 2
  Three: 4
  Two: 3
  One: 2
  Four: 5
  Five: 6
  Six: 7
....
asawyer
  • 17,642
  • 8
  • 59
  • 87
  • Thank you for taking the time to do this, lot for me to learn in here and I will definitely look into it and learn from it so it is appreciated. I would ask about, given the 12 chunk data, given the index creation, we are creating essentially 12 indexes? For each chunk of data, that is a lot of indexes if the data set is large. For example, I tested with 180 chunks of data, that is 2,160 indexes and we will use 180 of them? I mean, modern computers maybe it is not important, but seems like a lot of data creation? –  Aug 28 '20 at 20:56
  • Not sure I explained that well, but in your example, assuming it would continue the index output...I would take Index 1, and I would take Index 13 for the next set, and so on, but the indexes between 1 and 13, could not be used for the purposes that I need? –  Aug 28 '20 at 20:59
  • If I understand what your asking the extra 6 items per word set are discarded and only exist as part of the input split. – asawyer Aug 28 '20 at 22:14