Find and print all numbers in a comma-delimited string using sscanf I want to find all of the whole numbers in a string that starts with { and ends with }, and uses , to separate each number.

I use sscanf to find and match each item in the string—for example:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

int main(void)
{

    const char *str =  "{0,1,22,333,4,55,6,77777,8}";
    int tmp, tmp2, tmp3, number;

    printf("filtered number:");

    tmp = 0;
    while(1)
    {
        if( *str == '{' )
        {
            /* begin of current str */
            tmp2 = 0;
            tmp3 = 0;
        }

        if( *str == ',' || *str == '}' )
        {
            /* found a number now */
            tmp++;

            /* get how far the ptr move */
            //printf("ptr moved step: %d\n", tmp2);

            /* get the digit len */
            tmp3 = tmp2 - tmp3 - 1;

            /* get the number */
            sscanf(str-tmp3, "%d", &number);
            printf("%d ", number);

            /* record current ptr possition */
            tmp3 = tmp2;
        }

        if ( *str == '}' ) break;

        str++;
        tmp2++;
    }
    printf("\n");
    printf("howm any number found: %d\n", tmp);

    return 0;
}

Questions:

Any risk in my code?
Is there a smarter way to do this? Can my code be optimized?

Some general comments about your current code:

You just dumped everything in the main method. This makes your code difficult to read, and more importantly, prevents it from being re-used. You should have factored the logic out into a separate method, which you would then call from main. This method would take the input string as a parameter, and then either do the printing directly, or use output parameters to return the parsed data.
Your variable names are…terrible. tmp, tmp2, tmp3 provide no semantic information whatsoever about the purpose of that variable. All that tells me is that they are "temporary", but all local variables are temporary! What temporary values do these variables hold? Choosing meaningful names for your variables goes a long way towards making your code readable—and bug free!
Your commenting style is inconsistent. You can use either C89-style comments (/* ... */) or C99-style comments (// ...), but whichever one you use, stick with it! I would prefer the C99-style comments, since these are easier to write, less error-prone when editing, and ubiquitously supported by C compilers (even Microsoft's, which has notoriously incomplete support for other C99 syntax).
Speaking of comments, it looks like you are using comments to remove your debugging code. A better way to do that would be to use conditional compilation, based on the definition of a DEBUG symbol (defined for debug builds; undefined otherwise). For example:

#ifdef DEBUG
    printf("ptr moved step: %d\n", tmp2);
#endif  // DEBUG

You could go one better by defining a TRACE or DEBUG_OUTPUT (or whatever you want to call it) macro, and then using this each time you want to print debugging output. This makes your code cleaner because you don't have to have all the preprocessor statements littered throughout.
Prefer to declare variables at the point of initialization. This allows you to limit the scope and prevents the inadvertent use of uninitialized variables.

Speaking of uninitialized variables, if you had been compiling with warnings enabled (by passing -Wall or a similar option, depending on your compiler), you would have seen a warning that tmp2 and tmp3 may be uninitialized at the point of use. Since the use of uninitialized variables is undefined behavior, this is something that you want to avoid at all costs.
There are typos, grammatical errors, and inconsistent formatting (trailing spaces, etc.) in your output strings. This makes your code—and even your finished application—look unprofessional. Even if English is not your first language, you should carefully proofread any output strings that the user is going to see. Maybe even get someone to look over them. (You know, like in a Code Review! :-p)
Combine back-to-back calls to printf into a single call. For example, this code:

printf("\n");
printf("howm any number found: %d\n", tmp);

should instead be written simply as (grammar fixed, too):

printf("\nTotal numbers found: %d\n", tmp);

printf is an extremely slow function, so for both readability and performance, you should avoid calling it multiple times when you don't actually need to!

A major improvement using strtod:

You have made this way too difficult! You don't need to write nearly as much of your own parsing code if you leverage the power of the standard library, which has powerful string-manipulation functions.

One of the comments mentions using strtod. That is an option, but the sample input you provided suggests that these are integer values, not floating point values. Therefore, strtol would be a better choice. By passing 0 as the base (third parameter), it will automatically detect the correct representation for the value, which is the most flexible.

Here is my first pass at rewriting your code:

#include <stdlib.h>
#include <stdio.h>
#include <assert.h>

void FindAndPrintNumbers(const char *psz)
{
   // The beginning of the list should be marked by an open curly brace.
   assert(*psz == '{');

   // Parse and print out each of the numbers in the list,
   // keeping track of how many were found in all.
   printf("Filtered number(s): ");
   int numbersFound = 0;
   while (*psz++ != '}')
   {
      ++numbersFound;

      char* pszNext;
      printf("%ld ", strtol(psz, &pszNext, 0));
      psz = pszNext;

      // The next character should either be a comma or closed curly brace.
      // If not, the input string is malformed.
      assert((*psz == ',') || (*psz == '}'));
   }

   printf("\nTotal numbers found: %d\n", numbersFound);
}

Note that I tend to use semantic Hungarian prefixes for my variables—in particular, psz means "pointer to a string that is zero-terminated. You can either like this style or hate this style. Rename them if desired; it won't offend me.

Also, I had to introduce a temporary pszNext variable because the strtol function is not const-correct. In particular, although the first argument is a const pointer, the second parameter is a non-const pointer-to-a-pointer. This is a defect in the C standard library, and you just have to work around it, making sure not to inadvertently invoke undefined behavior by modifying the string through that non-const pointer! This makes the code uglier than I'd like, but the only other choice would be a cast, and that's arguably even uglier!

Finally, my code just assumes that the input string will be in the format you've shown. That is, it will begin with an open curly brace ({), will contain an arbitrary sequence of integer literals separated by commas (,) with no spaces, and will end with a closed curly brace (}). There are assertions here to enforce these assumptions, but the behavior is left formally undefined if a malformed input string is passed in. If you want to be more robust in what you handle, you will need to add extra error checks. Your original code wasn't robust in the face of invalid inputs, either, so I assumed this wasn't important.


Determining the state of matter for some substances I am new to React and still getting a handle on props and state. I tried
 to adopt the example of lifting state from
the React Docs
 and add the feature of different criteria by liquid type for evaluating these values.

Here is the JSBin version.

const liquidNames = {
  w: "Water",
  eth: "Ethanol"
};

var liquidInventory = {ethanol: {
  name: "Ethanol",
  freezing: -173.2,
  boiling: 173.1
}, "water": {
  name: "Water",
  freezing: 0,
  boiling: 212
}};

//test function to determine 'state of matter'
function LiquidState(props){
  var liquid = props.liquidType;
  if (props.temperature >= liquidInventory[liquid].boiling){
    return
 Boiling {liquid}

  } else if (props.temperature <= liquidInventory[liquid].freezing){
    return
 Frozen {liquid}

  } else {
    return
 You have liquid {liquid}

  }
}

//component to present the inputs and values of each water and ethanol
class TemperatureInput extends React.Component {
  constructor(props) {
    super(props);
    this.handleChange = this.handleChange.bind(this);
  }

  handleChange(e) {
    this.props.onTemperatureChange(e.target.value);
  }

  render() {
    const temperature = this.props.temperature;
    const liquidType = this.props.liquidType;
    return (

      <fieldset>
        <legend>Enter Temperature for {liquidType}: </legend>
        <input value={temperature}
          onChange={this.handleChange} />
          < LiquidState liquidType={liquidType} temperature={temperature} />
      </fieldset>

    );
  }
}

//parent Temperature component that holds overall program state
class Temperature extends React.Component {
  constructor(props) {
    super(props);
    this.testWater = this.testWater.bind(this);
    this.testEthanol = this.testEthanol.bind(this);
    this.state = {
      temperature: '',
      liquidType: ''
    };
  }

  testWater(temperature) {
    this.setState({temperature})
  }

  testEthanol(temperature) {
    this.setState({temperature})
  }

  render() {
    const liquidType = this.state.liquidType;
    const temperature = this.state.temperature;
    return (

        <TemperatureInput
          temperature={temperature}
          liquidType={'water'}
          onTemperatureChange={this.testWater}  />
         <TemperatureInput
          temperature={temperature}
           liquidType={'ethanol'}
          onTemperatureChange={this.testEthanol}  />

    );
  }
}

ReactDOM.render(
  <Temperature />,
  document.getElementById('root')
);

Works well!

This:

const liquidType = this.state.liquidType;
const temperature = this.state.temperature;

Can be written as:

const { liquidType, temperature } = this.props;

This:

function LiquidState(props){
  var liquid = props.liquidType;
  if (props.temperature >= liquidInventory[liquid].boiling){
    return
 Boiling {liquid}

  } else if (props.temperature <= liquidInventory[liquid].freezing){
    return
 Frozen {liquid}

  } else {
    return
 You have liquid {liquid}

  }
}

Can be written as:

const LiquidState = ({ liquidType, temperature }) => {
  if (temperature >= liquidInventory[liquidType].boiling){
    return
 Boiling {liquidType}

  } else if (temperature <= liquidInventory[liquidType].freezing){
    return
 Frozen {liquidType}

  } else {
    return
 You have liquid {liquidType}

  }
}

It's not much, but it's been useful to me.

I think lifting the state up is mainly important, if you're actually using the state in the high level component or one of its child components.

In this case, the value in the input is passed on to every input. If you instead keep state local to the component, you can type different values for each liquid.

I've just made my own attempt to learn something myself. Maybe there's one more useful thing hiding in there:

var liquids = {
  ethanol: {
    name: "Ethanol",
    states: [
      {
        minTemperature: 173.1,
        maxTemperature: Infinity,
        state: 'Boiling'
      },
      {
        minTemperature: -173.2,
        maxTemperature: 173.1,
        state: 'Liquid'
      },
      {
        minTemperature: -(Infinity),
        maxTemperature: -173.2,
        state: 'Frozen'
      }
    ]
  },
  water: {
    name: "Water",
    states: [
      {
        minTemperature: 212,
        maxTemperature: Infinity,
        state: 'Boiling'
      },
      {
        minTemperature: 0,
        maxTemperature: 212,
        state: 'Liquid'
      },
      {
        minTemperature: -(Infinity),
        maxTemperature: 0,
        state: 'Frozen'
      }
    ]
  }
}

class TemperatureInput extends Component {
  constructor(props) {
    super(props)
    this.state = {
      temperature: 0
    }
  }

  onChangeTemperature(event) {
    this.setState({ temperature: parseFloat(event.target.value) })
  }

  renderLiquidState(liquid, temperature) {
    for(var i = 0; i < liquid.states.length; i++) {
      if(liquid.states[i].minTemperature < temperature && temperature <= liquid.states[i].maxTemperature) {
        return {liquid.states[i].state} {liquid.name}
      }
    }
  }

  render() {
    const { liquid } = this.props
    const { temperature } = this.state

    return (

        <fieldset>
          <legend>
            Enter Temperature for {liquid.name}:
          </legend>
          <input
            value={temperature}
            onChange={this.onChangeTemperature.bind(this)}
          />

          {this.renderLiquidState(liquid, temperature)}
        </fieldset>

    )
  }
}

class Temperature extends Component {
  render() {
    return (

        <TemperatureInput liquid={liquids.water} />
        <TemperatureInput liquid={liquids.ethanol} />

    )
  }
}


console.log helper function I've seen the code for this function in John Resig's book 'Secrets of JavaScript Ninja' yesterday.

I considered it useful at once because I've disliked repeatedly typing console.log for some time.

Here's my implementation of a helper function to reduce typing:

// -- Function definition -------------
// Helper function for to simplify the
//  usage of console.log().
// -- Parameter -----------------------
//  Pass as many parameter as you like.
//  Separate them with commas.
function l() {
  console.log.apply(console, arguments);
}

Usage examples:

var car = new Car(180, 'Fiat');
var someArbitraryNumber = 248;

l('My car', car, ' - ', someArbitraryNumber);

l(123, 'abc', false, 248, 'A', ' > --- < ')

Output:

My car Car { color="red",  maxFuel=40,  speed="180 km"}  - 248

123 abc false 248 A  > --- <

What's great about it: instead of typing console.log(...), you can type l(...); much quicker! This simple helper behaves exactly like console.log. For example, the parameters can be any mixture of types, and objects printed on your browser console can be expanded to see their content, they aren't converted to string representations.

If there are any ideas to improve / enhance the function I would be pleased to get that feedback.

l is a terrible name for anything. Depending on the font used, it may be indistinguishable from the number one. It's generally recommended to avoid using the letter "l" for anything. (In some languages this recommendation is part of the official style guide.)

log would be better, still short, and with a more obvious meaning than just "l".


Optimize function that does phrase-search and lists them I am writing a long piece of code, which is taking way too long to execute. I used  cProfile on the code, I found that the following function is called 150 times and takes 1.3 seconds per call, leading to around 200 seconds to this function alone. The function is -

def makeGsList(sentences,org):
    gs_list1=[]
    gs_list2=[]
    for s in sentences:
        if s.startswith(tuple(StartWords)):
            s = s.lower()
            if org=='m':
                gs_list1 = [k for k in m_phrases if k in s]
            if org=='h':
                gs_list1 = [k for k in h_phrases if k in s]
            for gs_element in gs_list1:
                gs_list2.append(gs_element)
    gs_list3 = list(set(gs_list2))
    return gs_list3

The code is supposed to take a list of sentences and a flag org. Then it goes through each line, checks if it starts with any of the words present in the list StartWords, and then lower-cases it. Then, depending on the value of org, it makes a list of all phrases in the current sentence which are also present in either m_phrases or h_phrases. It keeps appending these phrases to another list gs_list2. Finally it makes a set of gs_list2 and returns it.

Can someone give me any suggestion as to how I can optimize this function to reduce the time taken to execute?

Some examples -

StartWords = ['!Series_title','!Series_summary','!Series_overall_design','!Sample_title','!Sample_source_name_ch1','!Sample_characteristics_ch1']

sentences = [u'!Series_title\t"Transcript profiles of DCs of PLOSL patients show abnormalities in pathways of actin bundling and immune response"\n', u'!Series_summary\t"This study was aimed to identify pathways associated with loss-of-function of the DAP12/TREM2 receptor complex and thus gain insight into pathogenesis of PLOSL (polycystic lipomembranous osteodysplasia with sclerosing leukoencephalopathy). Transcript profiles of PLOSL patients\' DCs showed differential expression of genes involved in actin bundling and immune response, but also for the stability of myelin and bone remodeling."\n', u'!Series_summary\t"Keywords: PLOSL patient samples vs. control samples"\n', u'!Series_overall_design\t"Transcript profiles of in vitro differentiated DCs of three controls and five PLOSL patients were analyzed."\n', u'!Series_type\t"Expression profiling by array"\n', u'!Sample_title\t"potilas_DC_A"\t"potilas_DC_B"\t"potilas_DC_C"\t"kontrolli_DC_A"\t"kontrolli_DC_C"\t"kontrolli_DC_D"\t"potilas_DC_E"\t"potilas_DC_D"\n',  u'!Sample_characteristics_ch1\t"in vitro differentiated DCs"\t"in vitro differentiated DCs"\t"in vitro differentiated DCs"\t"in vitro differentiated DCs"\t"in vitro differentiated DCs"\t"in vitro differentiated DCs"\t"in vitro differentiated DCs"\t"in vitro differentiated DCs"\n', u'!Sample_description\t"DAP12mut"\t"DAP12mut"\t"DAP12mut"\t"control"\t"control"\t"control"\t"TREM2mut"\t"TREM2mut"\n']

h_phrases = ['pp1665', 'glycerophosphodiester phosphodiesterase domain containing 5', 'gde2', 'PLOSL patients', 'actin bundling', 'glycerophosphodiester phosphodiesterase 2', 'glycerophosphodiester phosphodiesterase domain-containing protein 5']

m_phrases are similar. Assume in this case, org=h.

Regarding sizes -

The length of both lists h_phrases and m_phrases is around 250,000. And each element in the lists is on an average 2 words long. The list of sentences is around 10-20 sentences long and I have provided an example list to give you the idea of how big each sentence can be.

To improve the performance of code, a good plan is to:

Identify the hotspots, for example using profiling.
Prepare a representative test case and measure its performance.
Experiment with changes and see which ones improve the performace.

Step (2) is very important — if we don't have a representative test case then we might end up measuring the wrong thing. In this case I understand from the post that in the real program:

There are few sentences (tens), mostly 100 words or fewer.
There are many phrases (250,000), mostly with 5 words or fewer.
The matches are quite sparse (most sentences have no phrases).
The sentences change between calls to makeGsList but the phrases remain the same for each call.

So my test case is going to use your sentences list (8 elements is reasonably close to "10–20"), but I'm going to have to make my own phrases list, because you've only given me 7 elements which is nowhere near to 250,000.

>>> words = [line.strip().lower() for line in open('/usr/share/dict/words')]
>>> len(words)
235886
>>> from random import sample, randrange
>>> m_phrases = [' '.join(sample(words, randrange(1, 6))) for _ in range(250000)]

and then I'll time ten executions of makeGsList using Python's timeit module:

>>> from timeit import timeit
>>> timeit(lambda:makeGsList(sentences, 'm'), number=10)
2.7161848249379545

This is roughly similar to your reported results, so it might be representative enough. Now to try some changes.

First, let's tidy the code.

Instead of converting StartWords to a tuple for each sentence, convert it just once.
Instead of taking an org argument (and then checking it for each sentence), take a phrases argument.
Instead of maintaining a list gs_list2 and then converting it to a set to remove duplicates, maintain a set instead.

Revised code (this isn't noticeably faster, but it's in a better shape to work on):

def makeGsList2(sentences, phrases):
    "Return list of elements of phrases that appear in any of the sentences."
    start_words = tuple(StartWords)
    gs_set = set()
    for s in sentences:
        if s.startswith(start_words):
            s = s.lower()
            for phrase in phrases:
                if phrase in s:
                    gs_set.add(phrase)
    return list(gs_set)

It's clear that we have to avoid the loop over the phrases for each sentence. What we can do instead is to pre-process the phrases into a data structure that we can apply once to each sentence. What we need here is the Aho–Corasick algorithm. This isn't built into Python, but we could try one of the packages from PyPI. In the code below I'm using the pyahocorasick package; see the documentation for details.

import ahocorasick

def make_automaton(phrases):
    """Return an ahocorasick.Automaton that matches any of the phrases."""
    automaton = ahocorasick.Automaton()
    for phrase in phrases:
        automaton.add_word(phrase, phrase)
    automaton.make_automaton()
    return automaton

def makeGsList3(sentences, automaton):
    "Return list of unique matches of the automaton against the sentences."
    start_words = tuple(StartWords)
    gs_set = set()
    for s in sentences:
        if s.startswith(start_words):
            gs_set.update(phrase for _, phrase in automaton.iter(s.lower()))
    return list(gs_set)

Building the automaton takes a few seconds but it can be done just once for each list of phrases:

>>> m_automaton = make_automaton(m_phrases)

And then the actual search is very quick:

>>> timeit(lambda:makeGsList3(sentences, m_automaton), number=10)
0.0021720759104937315

This is about 1,200 times faster than the code in the post! Of course, this result only stands up to the extent that my test case is representative of the real system.


File Utilities in Go I'm beginning to learn Go. I've decided to write some file utilities to help me with some bigger projects I intend to write. Coming from mainly a Python background, I'm sure there are plenty of stylistic and efficiency improvements to be made. I'm looking for general feedback, as I want to kick bad habits to the curb before they set it. Feedback such as:

Error Reporting
Use of defer in appendContent function
Testing Functions: Are there any libraries to help accomplish this, like how Python has unittest?

I use more comments than I usually would when programming because I'm still learning the language. The Usage: comments help me remember why I'm importing these libraries. Besides the specifically requested feedback, any and all criticism is welcome and appreciated!

Utilities

package main

import (
    "fmt"       // Usage: Printing errors and content of the files
    "io/ioutil" // Usage: Reading and writing files
    "os"        // Usage: Opening files and appending to them
)

func main() {
    testFunctions()
}

func testFunctions() {
    /*
        This method is solely for the purpose of testing the below methods
    */
    writeContent("test.txt", "Test Content")
    fmt.Println(getContent("test.txt"))
    appendContent("test.txt", "Test Content 2")
    fmt.Println(getContent("test.txt"))
}

func getContent(file string) string {
    /*
        Gets the content from the passed `file`
    */
    content, err := ioutil.ReadFile(file)
    if err != nil {
        fmt.Println(err)
        return "[NOT CONTENT]: AN ERROR OCCURED"
    }
    return string(content)
}

func writeContent(file string, content string) {
    /*
        Writes the passed `content`, creating a new file at `file`
    */
    formattedContent := []byte(content)
    err := ioutil.WriteFile(file, formattedContent, 0777)
    if err != nil {
        fmt.Println(err)
    }
}

func appendContent(file string, content string) {
    /*
        Appends the passed `content` to the end of the passed `file`
    */
    fileToAppend, err := os.OpenFile(file, os.O_APPEND|os.O_WRONLY, 0600)
    if err != nil {
        panic(err)
    }
    defer fileToAppend.Close()
    _, err = fileToAppend.WriteString(content)
    if err != nil {
        panic(err)
    }
}

Regarding testing:  Yes, it's in the standard library.  There are of course more libraries to help out more, but that's where you should likely start your journey.

For error reporting, well, it's a bit inconsistent at the moment?  There's both returning a value ([NOT CONTENT]...) as well as fmt.Println and panic too, all of which are not very appropriate for production usage.

The first of which can lead to serious debugging nightmares:  How do you know the file didn't contain that exact string for one?  This is always a problem if the valid values are mixed with error values (like how in most C APIs there are int return values with -1 for errors, while everything \$>= 0\$ are the "actual" return values in the non-error case)!  It's much better to keep them separate, which in Go could be done with separate types, or separate return values (which is what's done for errors).

The second one is perhaps less bad, but again, the error situation is just hidden, not actually dealt with.

The third one can be done, however it's usually reserved for programming errors, since not catching a panic call will simply abort the program (which is rarely what you want).

If in doubt, return an error in addition to any values: func getContent(string) (string, error) and writeContent/appendContent(string, string) error would be fine.  Maybe also see this blog post and this newer one for 1.13.

The defer to close files is pretty much required in my opinion.  Unless you can prove that nothing else between the opening of the file and the Close call can raise a panic, you pretty much have to use defer ...Close(), otherwise you risk leaking resources (open file handles).

For the comments, please have a look here:  Instead of comments simply add proper docstrings to the functions, that is good practice anyway and would certainly be appreciated by colleagues on bigger projects.

Maybe as a last note:  The permissions 0777 and 0600 are okay if those are the ones you want.  However, that might not be true in general.  Exactly how ioutil.WriteFile lets you specify them would be likely what you want in general.  If not, consider creating constants so that you don't have magic numbers inline with the code (e.g. const defaultFilePermissions = 0777 or something similar).


csview: A tiny utility to view csv files Seeing a csv in table form is nicer then viewing it as row text, for example the csv from Calculate food company sales for the year (with headers added by me) looks much nicer in this table form than in plain-text:

    Kind
 Brand
 Sales in 2014
 Sales in 2015

    Cereal
Magic Balls
2200
2344

    Cereal
Kaptain Krunch
3300
3123

    Cereal
Coco Bongo
1800
2100

    Cereal
Sugar Munch
4355
6500

    Cereal
Oats n Barley
3299
5400

    Sugar Candy
Pop Rocks
546
982

    Sugar Candy
Lollipop
1233
1544

    Sugar Candy
Gingerbud
2344
2211

    Sugar Candy
Respur
1245
2211

    Chocolate
Coco Jam
3322
4300

    Chocolate
Larkspur
1600
2200

    Chocolate
Mighty Milk
1234
2235

    Chocolate
Almond Berry
998
1233

    Condiments
Peanut Butter
3500
3902

    Condiments
Hot Sauce
1234
1560

    Condiments
Jelly
346
544

    Condiments
Spread
2334
5644

To view a .csv file, I just translate it into HTML and call the browser on it.

The code is short because the task is simple, but I feel like it could be written better:

"""
This utility shows a csv file in table format
by translating it to html and calling the browser on the newly created file.

The html file is not deleted after being viewed
and has name `original_file.split('.')[0] + '.html'`

Example usage:
  python3 csview.py example.csv
"""
import webbrowser
import sys

def csv_to_html(csv):
    START = '''
\n\n'''
    END = '''

'''

    lines = [line for line in csv.split("\n") if line]

    html_lines = ["" + lines[0].replace(',', '	\n') + '	'] +\
                 ["" + line.replace(',', '	\n') for line in lines[1:]]
    body = '
\n' + '\n
\n\n
\n'.join(html_lines)
    return START + body + END

if __name__ == "__main__":
    html_filename = sys.argv[1].split('.')[0] + '.html'
    with open(html_filename, "w+") as out_file:
        with open(sys.argv[1]) as in_file:
            out_file.write(csv_to_html(in_file.read()))
    webbrowser.open(html_filename)

Constants
They are better defined outside of functions; there is no need in reinitializing them each call.
Also they are not balanced, fixing it will avoid a weird body = '<tr>\n' + ....
Files management
Just in case the path of the input file contained a dot, I recommend you use rsplit('.', 1).
Also you should try to open the input file before the ouput one. Just in case an error happen during the access to the file, you won't create an empty output file.
And since you're dealing with CSV files, why not use the csv module? It will allow you to handle edge-case data (such as fields containing comas or newlines) easily.
Wrap stuff into tags
Since you're outputing HTML, it may be a better idea to build it as such: by wrapping stuff into tags.
You could define a PATTERN = "<{tag}>{{}}</{tag}>"  and a formatter:
def formatter(tag):
    return PATTERN.format(tag=tag).format

map that formatter at will to wrap stuff into tags.
Proposed improvements
import webbrowser
import sys
import csv

PATTERN = "<{tag}>{{}}</{tag}>"

def formatter(tag):
    return PATTERN.format(tag=tag).format

def csv_to_html(csv_file):
    reader = csv.reader(csv_file)

    header = formatter('tr')(
        '\n'.join(map(formatter('th'), next(reader)))
    )

    data = '\n'.join(
        map(formatter('tr'),
            '\n'.join(map(formatter('td'), row))
            for row in reader
        )
    )

    return formatter('table')('\n'.join((header, data)))

if __name__ == "__main__":
    csv_filename = sys.argv[1]
    html_filename = csv_filename.rsplit('.', 1)[0] + '.html'
    with open(csv_filename) as in_file:
        with open(html_filename, "w+") as out_file:
            out_file.write(csv_to_html(in_file))
    webbrowser.open(html_filename)


Scrape HTML into separate files for each chapter I have written a program to read HTML file, find particular text, write text from that particular text to next particular text in a newly opened file. I am interested in code review. I am very new PHP.

<?php

$FILE = "testing.html";
$count = 0;
$directoryName = explode('.',$FILE)[0];
$TAG = "/x-berschrift-1--nur-f-r-Header-/";

function readingFile($FILE)
{
    //Creates a directory on the file name to store segmeted chapters from $FILE
    global $directoryName;
    mkdir($directoryName);
    $file_handle = fopen($FILE, "r");
    chdir($directoryName);
    $matchTag = False;
    while (!feof($file_handle))
     {
           $line = fgets($file_handle);
           if(checkingMatchTag($line))
           {
                if($matchTag){
                    closeWriteFile($writeHandle);
                    incrementCounter();
                }
                else
                {
                    $matchTag = True;
                }
                $writeHandle = openWriteFile();
                segmentContent($writeHandle, $line);
           }
           elseif($matchTag)
           {
               segmentContent($writeHandle, $line);
           }
     }
}

function checkingMatchTag($line)
{
    global $TAG;
    if(preg_match($TAG, $line))
    {
        return True;
    }
    return False;
}

function openWriteFile()
{
    global $directoryName, $count, $FILE;
    $writingFile = fopen($directoryName.$count.'.'.explode('.',$FILE)[1], 'w');
    return $writingFile;
}

function segmentContent($writeFileHandle, $line)
{
   fwrite($writeFileHandle, $line);
}

function closeWriteFile($file)
{
    fclose($file);
}

function incrementCounter()
{
    global $count;
    $count++;
}

readingFile($FILE);

Don't use globals in this case
To start off, global variables should generally be avoided.  Often times configurations will use global constants, but using global variables can have implications to the namespace.  See pacmaninbw's answer for a little more explanation on that.
With that said,  if you wanted to keep the values global please use constants.
define('FILE', "testing.html");
define('TAG', "/x-berschrift-1--nur-f-r-Header-/");
define('DIRECTORY', "testing");

Define global constants like this at the beginning of your file.

Simplify (and remove) your functions
Next up are your functions:
function checkingMatchTag($line)
{
    return preg_match(TAG, $line);
}

function openWriteFile($directory, $count, $extension)
{
    return fopen($directory . $count . '.' . $extension, 'w');
}

function closeWriteFile($file)
{
    fclose($file);
}

function segmentContent($writeFileHandle, $line)
{
    fwrite($writeFileHandle, $line);
}

As you can see, I simplified your functions for you.  They perform the same, but have less clutter involved. In checkingMatchTag I left in the call to TAG in to show you how to call global constants.
Anyway, the point is each of these functions can be reduced to one simple line.  So why are you making a ton of functions for one line of basic code?  I would advise you get rid of each of these functions and simply use the line of code.

Misc changes
Make sure the directory is not there by using this code:
if (!file_exists(DIRECTORY)) {
    mkdir(DIRECTORY);
}

Use the same naming convention for everything. Look at your perimeters. Look at your spacing.  Look at your {}.  There are differences in all of them.


Delete directory tree function I made this function to delete a directory with all its contents recursively, will it work as expected in a production environment ? is it safe ? I don't want to wake up one day with /home contents is gone :D

public static function delTree($dir) {
    if(!is_dir($dir)){return false;};

    $files = scandir($dir);if(!$files){return false;}
    $files = array_diff($files, array('.','..'));

    foreach ($files as $file) {
        (is_dir("$dir/$file")) ? SELF::delTree("$dir/$file") : unlink("$dir/$file");
    }

    return rmdir($dir);
}

Note: I use this function internally, meaning there are no client parameters like directory names is taken from the client before I call it, so there is no chance for traversal attacks, and I check the base path with another function before I call it, for example to delete a client folder I do something like this

$clientsFolderPath = $_SERVER['DOCUMENT_ROOT'] . "/../clients"
$clientFolderPath = "$clientsFolderPath/$clientId";

$realBase = realpath($clientsFolderPath);
$realClientDir = realpath($clientFolderPath);
if ( !$realBase || !$realClientDir || strpos($realClientDir, $realBase) !== 0 ){
    //error, log , and exit;
} else {
    ExtendedSystemModel::delTree($clientFolderPath);
}

As indicated by the scattered comments on https://stackoverflow.com/q/3349753/2943403, your approach is trustworthy.

scandir() has an advantage over glob() (which is normally handy when trying to ignore . and ..) because glob() will not detect hidden files.

The RecursiveIterator methods are powerful, but it is my opinion that fewer developers possess the ability to instantaneously comprehend all of the calls and flags (and I believe that should weigh in on your decision).

As for your snippet, I would like to clean it up a little.

public static function delTree($dir) {
    if (!is_dir($dir)) {
        return false;
    }

    $files = scandir($dir);
    if (!$files) {
        return false;
    }
    $files = array_diff($files, ['.', '..']);

    foreach ($files as $file) {
        if (is_dir("$dir/$file")) {
            SELF::delTree("$dir/$file");
        } else {
            unlink("$dir/$file");
        }
    }

    return rmdir($dir);
}

I don't use ternary operators when I am not assigning something in that line.  For this reason, a classic if-else is cleaner in my opinion.


Speedily Read and Parse Data As of now, I am using this code to open a file and read it into a list and parse that list into a string[]:

string CP4DataBase = "C:\\Program\\Line Balancer\\FUJI DB\\KTS\\KTS - CP4 - Part Data Base.txt";
CP4DataBaseRTB.LoadFile(CP4DataBase, RichTextBoxStreamType.PlainText);
string[] splitCP4DataBaseLines = CP4DataBaseRTB.Text.Split('\n');
List<string> tempCP4List = new List<string>();
string[] line1CP4Components;

foreach (var line in splitCP4DataBaseLines)
    tempCP4List.Add(line + Environment.NewLine);

string concattedUnitPart = "";
foreach (var line in tempCP4List)
{
    concattedUnitPart = concattedUnitPart + line;
    line1CP4PartLines++;
}

line1CP4Components = new Regex("\"UNIT\",\"PARTS\"", RegexOptions.Multiline)
                    .Split(concattedUnitPart)
                    .Where(c => !string.IsNullOrEmpty(c)).ToArray();

I am wondering if there is a quicker way to do this. This is just one of the files I am opening, so this is repeated a minimum of 5 times to open and properly load the lists.

The minimum file size being imported right now is 257 KB. The largest file is 1,803 KB. These files will only get larger as time goes on as they are being used to simulate a database and the user will continually add to them.

So my question is: is there a quicker way to do all of the above code?

The part of the code that makes it really slow is this:

string concattedUnitPart = "";
foreach (var line in tempCP4List)
{
  concattedUnitPart = concattedUnitPart + line;
  line1CP4PartLines++;
}

You should not concatenate large strings like that. The string gets longer and longer for each iteration, and it's copied into a new string each time.

If you Read a file that is 1.8 MB, that consists of lines which varies between 50 and 100 characters, you will have been copying about 10 000 MB of data before you have the result.

Also, it scales very badly, so when the files grow it will grow slower at an exponential rate. To handle a file that is 5 MB you will be copying about 80 000 MB of data.

James suggestion to do a replace seems to be a good option. If you want to split and join, you can use the String.Join method:

string[] splitCP4DataBaseLines = CP4DataBaseRTB.Text.Split('\n');
string concattedUnitPart = String.Join(Environment.NewLine, splitCP4DataBaseLines) + Environment.NewLine;


Creating a thumbnail of a PDF using Cairo, Poppler and Rust I want to create thumbnails from PDF files – specifically, a thumbnail of the first page. Currently I use pdftocairo on the command line:

$ pdftocairo myfile.pdf -png -singlefile -scale-to-x 400 -scale-to-y -1

but I wanted to see if I could implement this without shelling out to an external tool. I've written a function in Rust that links to poppler, takes the path to a PDF file, and a path to write a thumbnail to. It reads the first page of the PDF, renders it as a PNG, and saves it to the thumbnail path.

The function seems to work in the handful of PDFs I've tested, and I have a larger corpus I can test on if I decide to use this code.

I'd be interested in knowing:

Is this idiomatic Rust, especially the error handling? I don't have much experience writing Rust and I've been doing everything on my own, so my knowledge of what counts as "good" Rust code is quite limited.
Are there any bugs? I think I've covered the happy path and all the errors (thanks Rust!), but I might have missed something less obvious.
Is there a better way of doing this in Rust? I'm not tied to Cairo or Poppler, and if there's a Rust-native PDF library or similar that can do this sort of thing, I'd have a look at that instead.

extern crate cairo;
extern crate glib;
extern crate poppler;

use std::convert::From;
use std::fs::File;
use std::result::Result;

use cairo::{Context, Format, ImageSurface};
use poppler::{PopplerDocument, PopplerPage};

#[derive(Debug)]
pub enum ThumbnailError {
    GlibError(glib::error::Error),
    NoPagesError,
    CairoError(cairo::Status),
    CairoIoError(cairo::IoError),
    IoError(std::io::Error),
}

impl From<glib::error::Error> for ThumbnailError {
    fn from(err: glib::error::Error) -> Self {
        ThumbnailError::GlibError(err)
    }
}

impl From<cairo::Status> for ThumbnailError {
    fn from(status: cairo::Status) -> Self {
        ThumbnailError::CairoError(status)
    }
}

impl From<cairo::IoError> for ThumbnailError {
    fn from(err: cairo::IoError) -> Self {
        ThumbnailError::CairoIoError(err)
    }
}

impl From<std::io::Error> for ThumbnailError {
    fn from(err: std::io::Error) -> Self {
        ThumbnailError::IoError(err)
    }
}

/// Create a JPEG thumbnail from the first page of a PDF.
fn create_thumbnail(pdf_path: &str, out_path: &str) -> Result<(), ThumbnailError> {

    // Assume the PDF is not password protected.
    let doc: PopplerDocument = PopplerDocument::new_from_file(pdf_path, "")?;

    // Note: PDF pages are 0-indexed
    let page: PopplerPage = match doc.get_page(0) {
        Some(p) => p,
        None => return Err(ThumbnailError::NoPagesError),
    };

    let (width, height) = page.get_size();

    let mut surface = ImageSurface::create(
        Format::Rgb24,
        width as i32,
        height as i32)?;

    // Draw a white background to start with.  If you don't, any transparent
    // regions in the PDF will be rendered as black in the final image.
    let ctxt = Context::new(&mut surface);
    ctxt.set_source_rgb(1.0 as f64, 1.0 as f64, 1.0 as f64);
    ctxt.paint();

    // Draw the contents of the PDF onto the page.
    page.render(&ctxt);

    let mut f: File = File::create(out_path)?;
    surface.write_to_png(&mut f)?;

    Ok(())
}

fn main() {
    let pdf_path = "myfile.pdf";
    let out_path = "thumbnail.png";

    match create_thumbnail(pdf_path, out_path) {
        Ok(_) => println!("Created thumbnail of {} at {}", pdf_path, out_path),
        Err(err) => println!("Something went wrong: {:?}", err),
    };
}
```

This seems pretty good to me. I didn't notice any bugs, and I don't have knowledge of what pdf crates are available, so I just have a few points about idiomatic Rust.

To start, I just want to note that when I tried to create this project locally to work on, I had to fiddle around with the versions of the crates to get a working combination, which makes it an especially good idea to include what versions you're using somewhere in your question.

You can simply remove all uses of extern crate some_crate, since they aren't needed anymore in rust 2018 edition (the default by now).

I like the use of an error enum. You could consider using something like the crate thiserror to derive Display and From instances for the enum. (This just reduces boilerplate.)

I often like having type Result<T, E = MyErrorType> = std::result::Result<T, E>;, especially for a library, which lets me write Result<()> to use my error or Result<(), OtherError> to use another.

pub fn create_thumbnail(pdf_path: &Path, out_path: &Path) -> Result<()> {
    let doc = PopplerDocument::new_from_file(pdf_path, "")?;

    let page = doc.get_page(0).ok_or(Error::NoPagesError)?;

    let (width, height) = page.get_size();

    let surface = ImageSurface::create(Format::Rgb24, width as i32, height as i32)?;

    let ctxt = Context::new(&surface);
    ctxt.set_source_rgb(1.0, 1.0, 1.0);
    ctxt.paint();

    page.render(&ctxt);

    let mut f = File::create(out_path)?;
    surface.write_to_png(&mut f)?;

    Ok(())
}

You should use Paths when you're representing paths. You can use ok_or or ok_or_else to convert Option to Result, which works well with ?. Rust can infer the type of your floats in set_source_rgb, but if you want to be explicit then you should use 1.0f64 rather than 1.0 as f64. I removed your comments just for space in the answer, but they are meaningful since they give reason and don't just restate the code.


Complex structure vs object This is not, strictly speaking, code review, but I think my question can be asked here.

This is a request to find out what seems to be the cleanest. This project is a blog engine that uses the file system.  There are still many things to do to improve the code (it's not great).  I have such a class article at initialization that will parse actual items and store it in a structure and CHI a cache system.  In CHI, I store a table of hash:

 push(
     @articles,
     {
         title   => $title,
         tags    => \@tags,
         date    => $dt,
         content => $content,
         link    => $url
     }
 );

I wonder if it would not be better to make a class item and store it in my cache as an object list:

 package MahewinBlogEngine::Article;

 use Moo;

 use Type::Params qw( compile );
 use Type::Utils;
 use Types::Standard qw( Object Str Int ArrayRef );

 has title => (
     is       => 'rw',
     isa      => Str,
     required => 1,
 );

 has tags  => (
     is       => 'rw',
     isa      => ArrayRef,
     required => 1,
 );

 has date  => (
     is       => 'rw',
     isa      => Object,
     required => 1,
 );

 has content => (
     is       => 'ro',
     isa      => Str,
     required => 1,
 );

 has link => (
    is       => 'ro',
    isa      => Str,
    required => 1,
 );

 1;

And:

 my $article =  MahewinBlogEngine::Article->new(
     title   => $title,
     tags    => \@tags,
     date    => $dt,
     content => $content,
     link    => $url
);

push(@articles, $article);

This makes objects more maneuverable, powerful and extensible.  It also allows you to add methods such as formatting or other things.  But at the moment, I see no specific methods.  It will simply be a class with attributes and accessors. So, I wonder what is best in terms of quality, performance and memory.

Another question: I think CHI already compresses, but do you think it would be wise to use Sereal and compress before storing in the CHI system cache?

The CHI uses many caching backends and you did not said which one you are using. Fastest is perl's internal arrays, then hashes and then any other implementations. CHI may use persistent files to caching, like Berkeley DB backend or one of mmap implementation to reduce memory usage. When you are using such caching to improve perfomance you may place cache files to tmpfs (or some of ramfs) on *nix systems (linux, *bsd).

So, when I'm storing a lot of data in cache and I need to reduce memory usage I prefer use BDB without CHI. Then I need speed and managing few data in cache I prefer perl's internal structures. In last case you need to remember that Perl uses optimistic memory allocation. Thats means that Perl reallocates and allocates new memory segments and never returns consumed memory back to the OS. It is very important when writing daemons and you need to limit somehow memory usage. In other cases you may use any caching system and do not worry about performance or memory usage. Just use Devel::NYTProf to find bottlenecks.


Dynamic Allocation and Reallocation of Memory This is a code snippet of a larger program. I tried to simplify it as much as possible.

This code identifies certain particles and stores them in a list which corresponds to a certain processor.
Besides that I count the number of particles per processor that are already in the list.
After the work is done I want to use the message-passing system MPI to send the particles in the list to other processors (not shown here).

I would like to know:

Is the process of allocation and deallocation of memory correct?
Is the code robust?
Can this be simplified?

Code:

#include <stdio.h>
#include <stdlib.h>
#include <time.h>

#define DIM 2

typedef struct{
    double m;
    double x[DIM];
    double v[DIM];
    int id;
} Particle;

typedef struct{
    double **list;
    int *counter;
} ParticleList;

void generateParticles(Particle *p, int N);
void buildList(Particle *p, ParticleList *plist, int numprocs, int N);

int main() {
    time_t t;
    srand((unsigned)time(&t));

    // Generate data, print data
    int N = 50;
    Particle *p = (Particle*)malloc(N*sizeof(*p));
    generateParticles(p, N);
    for(int i=0; i<N; i++){
        printf("id: %d m: %lf x: %lf %lf v: %lf %lf\n", p[i].id, p[i].m, p[i].x[0], p[i].x[1], p[i].v[0], p[i].v[1]);
    }

    // Fill lists of every processor
    int numprocs = 4;
    ParticleList plist;
    plist.list = malloc(sizeof(double*)*numprocs);
    for(int k=0; k<numprocs; k++)
        plist.list[k] = malloc(sizeof(double)*1);
    plist.counter = calloc(numprocs, sizeof(int));
    buildList(p, &plist, numprocs, N);

    for(int k=0; k<numprocs; k++){
        printf("Processor %d Particles in list %d\n", k, plist.counter[k]);
        for(int c=0; c<(DIM*plist.counter[k]); c++){
            printf("%lf ", plist.list[k][c]);
        }
        printf("\n");
    }

    // deallocate memory
    for(int k=0; k<numprocs; k++){
        free(plist.list[k]);
    }
    free(plist.list);
    free(plist.counter);
    free(p);
    return 0;
}

void buildList(Particle *p, ParticleList *plist, int numprocs, int N){
    for(int k=0; k<numprocs; k++){
        for(int i=0; i<N; i++){
            printf("k %d, i %d\n", k, i);
            if(rand()%10 < 5){
                plist->counter[k] = plist->counter[k] + 1;
                plist->list[k] = realloc(plist->list[k], (DIM*plist->counter[k])*sizeof(plist->list[k][0]));
                int d=0;
                for(int j=(plist->counter[k] - 1)*DIM; j < (plist->counter[k] * DIM); j++){
                    plist->list[k][j] = p[i].x[d];
                    d++;
                }
            }
        }
    }
}

double frand(){
    return (double)rand() / (double)RAND_MAX;
}

void generateParticles(Particle *p, int N){
    for(int i=0; i<N; i++){
        for(int d=0; d<DIM; d++){
            p[i].x[d] = frand();
            p[i].v[d] = frand();
        }
        p[i].m = frand();
        p[i].id = i;
    }
}

Code would have benefited with allocateList(), allocateParticles(), freeList(), freeParticles() functions, rather than trying coding it all up in main().

allocation and deallocation of memory correct?

Allocations lack error checking.  This is the big issue as it also impacts proper de-allocation code.

plist->list[k] = realloc(plist->list[k], .... leak memory should the allocation fail.

DIM*plist->counter[k])*sizeof(plist->list[k][0]) could overflow int math, yet work well with size_t math.  See re-ordering below.

Robust-ness

Use size_t instead of int for indexing. As "code snippet of a larger program", the array sizes may be very large.  The best type for array indexing is size_t, an unsigned type.

// for(int d=0; d<DIM; d++){
for(size_t d=0; d<DIM; d++){
        p[i].x[d] = frand();
        p[i].v[d] = frand();
    }

Simplification

C does not require the cast nor () with sizeof object

// Particle *p = (Particle*)malloc(N*sizeof(*p));
Particle *p = malloc(N * sizeof *p);

// I prefer `sizeof` first, especially  for math that may benefit with `size_t` math.
//                   v---------------v all size_t math
Particle *p = malloc(sizeof *p * N * N);
// vs                v---v may overflow `int` math
Particle *p = malloc(N * N * sizeof *p);

Simplify coding, review and maintenance.  Is plist.list a double *?  Why bother attempting to co-coordinate the type of member list with the malloc() call which may be in another file?

// plist.list = malloc(sizeof(double*)*numprocs);
plist.list = malloc(sizeof *(plist.list) * numprocs);

// plist.counter = calloc(numprocs, sizeof(int));
plist.counter = calloc(numprocs, sizeof *(plist.counter));
// or
plist.counter = calloc(numprocs, sizeof plist.counter[0]);

The *1 is curious and lacks benefit

// plist.list[k] = malloc(sizeof(double)*1);
plist.list[k] = malloc(sizeof *(plist.list[k]));

buildList() would benefit with re-write - IMO, is is not simple to follow.  As code lacks guiding comments, I am left to guess how to simplify it without losing meaning.  Sample idea with some style improvements.

// Style issue: Avoid wrapping presentation code and horizontal scroll bars
void buildList(Particle *p, size_t n, ParticleList *plist, size_t numprocs) {
  for (size_t np=0; np<numprocs; np++) {
    for (size_t i=0; i<n; i++){
      printf("np %zu, i %zu\n", np, i);
      if (rand()%10 < 5) {
        size_t new_count = plist->counter[k] + 1;
        void *new_ptr = realloc(plist->list[np],sizeof plist->list[np][0] * DIM*new_count);
        if (new_ptr == NULL) {
          TBD_ErrorHandler();
          // ...
        }
        plist->list[k] = new_ptr;
        plist->counter[k] = new_count;
        // I think the following is the same - I had trouble following original code.
        for(size_t dm = 0; dm < DIM; dm++) {
          plist->list[pn][new_count + j] = p[i].x[dm];
        }
      } // endif
    }
  }
}

"l" in "%lf" not required - more of a style issue

// printf("id: %d m: %lf x: %lf %lf v: %lf %lf\n",
printf("id: %d m: %f x: %f %f v: %f %f\n",
  p[i].id, p[i].m, p[i].x[0], p[i].x[1], p[i].v[0], p[i].v[1]);

Sample ParticleList_free()

void ParticleList_free(ParticleList *pl) {
  if (pl) {
    if (pl->list) {
       for(int k=0; k<numprocs; k++) {
         free(pl->list[k]);
         pl->list[k] = NULL; // ***
       }
       free(pl->list);
       pl->list = NULL; // ***
    }
    free(pl>counter);
    pl>counter = NULL; // ***
    free(pl);
  }

  // Sample usage
  ParticleList *pl = ParticleList_alloc();
  buildList(p, pl, numprocs, N);
  ....
  ParticleList_free(pl);
  pl = NULL; // ***

*** Useful for debugging, but not needed for production code.  By setting to NULL, errant uses of pointers are typically more likely to be found.  YMMV.


Python decorator for retrying w/exponential backoff This is my first decorator in Python! I found some of it on the internet but have tweaked it to our needs.

Here is the couple concerns of mine:

Multiple python version compatibility
Is grabbing the self or arg[0] the best way to get the instance of the class?
Any other general improvements!

Here is the decorator.

import logging
import time

def retry_and_catch(exceptions, tries=5, logger=None, level=logging.ERROR, logger_attr=None, delay=0, backoff=0):
    """
    Retries function up to amount of tries.

    Backoff disabled by default.

    :param exceptions: List of exceptions to catch
    :param tries: Number of attempts before raising any exceptions
    :param logger: Logger to print out to.
    :param level: Log level.
    :param logger_attr: Attribute on decorated class to get the logger ie self._logger you would give "_logger"
    :param delay: initial delay seconds
    :param backoff: backoff multiplier
    """
    def deco_retry(f):
        def f_retry(*args, **kwargs):
            max_tries = tries
            d = delay
            exs = tuple(exceptions)
            log = logger
            while max_tries > 1:
                try:
                    return f(*args, **kwargs)
                except exs as e:
                    message = "Exception {} caught, retrying {} more times.".format(e.message, max_tries)

                    # Get logger from cls instance of function
                    # Grabbing 'self'
                    instance = args[0]
                    if not log and logger_attr and hasattr(instance, logger_attr):
                        log = getattr(instance, logger_attr, None)

                    if log:
                        log.log(level, message)
                    else:
                        print(message)

                    # Sleep current delay
                    if d:
                        time.sleep(d)

                        # Increment delay
                        if backoff:
                            d *= backoff
                    max_tries -= 1

            return f(*args, **kwargs)  # Final attempt will not catch any errors.
        return f_retry
    return deco_retry

Here is an example usage

class MyClass(object):
    _logger = logging.getLogger()

    @retry_and_catch([ValueError], logger_attr='_logger', delay=1, backoff=2)
    def my_func(self):
        raise ValueError('Unable to run my_func')

c = MyClass()
print(c.my_func())

First of all, when I start writing decorators of more than moderate complexity, and especially if they take parameters, I usually transition to writing them as classes - I find that easier to reason about and understand.

Second of all, in 100% of decorators I've ever written, I've wanted the decorator to look like the wrapped function.  To do this, just use functools.wraps().

Next, I think you have some weirdness in your api. I would expect logger to default to some default logger, and I would expect logger_attr to only be used to override whatever that logger is.  I also don't like that you're explicitly calling print in the decorator - instead you should always use the logger, and if they want one that just calls print, or if that is the default, you should provide that.  As an additional note, if logger_attr is a string then you can simplify the implementation a bit to look like this

logger = getattr(instance, attr_name, logger)
logger.log(level, message)

Next, you've limited yourself to some pretty static forms of backoff by making it a number. Instead, make it be a generator.  Then you can do something like this

def doubling_backoff(start):
    if start == 0:
        start = 1
        yield start
    while True:
        start *= 2
        yield start

def no_backoff(start):
    while True:
        yield start

and then in your decorator, it looks like this

backoff_gen = backoff(delay)
while max_tries > 1:
    try:
        return f(*args, **kwargs)
    except exceptions as e:
        message = "Exception {} caught, retrying {} more times.".format(e.message, max_tries)

        instance = args[0]
        logger = getattr(args[0], attr_name, logger)
        logger.log(level, message)

        time.sleep(delay)
        delay = next(backoff_gen)
        max_tries -= 1

This lets you add much more complex backoff algorithms as needed.

Lastly, I think your comments don't really add much value to the code - reading the code is self-explanatory. I also don't think that promoting the values to local variables is worthwhile, but given that this is somewhat time sensitive then it might be. I removed that, but YMMV.

My end result looked something like this:

import time
import logging
import functools

class DefaultLogger(object):

    def log(self, level, message):
        print(message)

def doubling_backoff(start):
    if start == 0:
        start = 1
        yield start
    while True:
        start *= 2
        yield start

def no_backoff(start):
    while True:
        yield start

class RetryAndCatch(object):

    def __init__(exceptions_to_catch, num_tries=5, logger=DefaultLogger(), log_level=logging.ERROR, logger_attribute='', delay=0, backoff=no_backoff)
        self.exceptions = exceptions_to_catch
        self.max_tries = num_tries
        self.tries = num_tries
        self.logger = logger
        self.level = log_level
        self.attr_name = logger_attribute
        self.delay = delay
        self.backoff = backoff

    def __call__(self, f):
        @functools.wraps(f)
        def retrier(*args, **kwargs):
            backoff_gen = self.backoff(delay)
            try:
                while self.tries > 1:
                    try:
                        return f(*args, **kwargs)
                    except self.exceptions as e:
                        message = "Exception {} caught, retrying {} more times.".format(e.message, self.tries)

                        instance = args[0]
                        self.logger = getattr(args[0], self.attr_name, self.logger)
                        self.logger.log(self.level, message)

                        time.sleep(self.delay)
                        self.delay = next(backoff_gen)
                        self.tries -= 1

                return f(*args, **kwargs)
            finally:
                self.tries = self.max_tries
        return retrier

As an aside, I wrote a somewhat similar decorator and asked about it here Memoizing decorator that can retry, but the backoff idea is pretty cool and I might incorporate that into mine.


Algorithm to generate this alignment pattern locations table for QR codes I'm looking for a way to generate the following the series of numbers given the version number on the left. For example, given "2", generate [6, 18]. The series of numbers is from the alignment pattern locations for different versions of QR codes.

 V2  6  18
 V3  6  22
 V4  6  26
 V5  6  30
 V6  6  34
 V7  6  22  38
 V8  6  24  42
 V9  6  26  46
V10  6  28  50
V11  6  30  54
V12  6  32  58
V13  6  34  62
V14  6  26  46  66
V15  6  26  48  70
V16  6  26  50  74
V17  6  30  54  78
V18  6  30  56  82
V19  6  30  58  86
V20  6  34  62  90
V21  6  28  50  72   94
V22  6  26  50  74   98
V23  6  30  54  78  102
V24  6  28  54  80  106
V25  6  32  58  84  110
V26  6  30  58  86  114
V27  6  34  62  90  118
V28  6  26  50  74   98  122
V29  6  30  54  78  102  126
V30  6  26  52  78  104  130
V31  6  30  56  82  108  134
V32  6  34  60  86  112  138
V33  6  30  58  86  114  142
V34  6  34  62  90  118  146
V35  6  30  54  78  102  126  150
V36  6  24  50  76  102  128  154
V37  6  28  54  80  106  132  158
V38  6  32  58  84  110  136  162
V39  6  26  54  82  110  138  166
V40  6  30  58  86  114  142  170

I've come up with the following Python code which generates the correct sequences, but contains an "exceptional" condition for V32, expressed in the if statement.

import math

def pattern(ver):
    w = 21 + ((ver - 1) * 4)       # Width
    h = 6                          # First item
    t = (w - 1) - 6                # Last item
    r = t - h                      # Distance between first and last
    n = math.ceil(r/28)            # Number of items in list - 1

    if (w == 145):
        intervals = 26             # Anomaly
    else:
        intervals = math.ceil(r/n) # Ceiling to nearest multiple of 2
        intervals = intervals + (intervals % 2)

    xs = [t]
    for m in (n-1) * [intervals]: xs.append(xs[-1] - m)
    xs.append(h)
    return list(reversed(xs))

for n in range(2, 41):
    print("Version {:2d}: {}".format(n, pattern(n)))

I'm looking for an algorithm where the magic branch & value is not needed The algorithm might not exist, but I'm hopeful.

I don't think such algorithm exists, because I looked quite hard and couldn't find it. It seems you have to follow the given table. Your formula approximates it pretty well. Too bad about the magic branch, but this shouldn't be a major concern anyway:
it's a small table that you only need to calculate once and can reuse later,
so any performance issues should be negligible in practice.

Coding style

The coding style can be improved.
Follow PEP8, the official coding style guide of Python,
for example, put 2 blank lines in front of top-level method definitions

Unnecessary parentheses

The parentheses around the multiplication are unnecessary here:

w = 21 + ((ver - 1) * 4)       # Width

It would be better to use more meaningful variable names,
for example no need to shorten version as ver.

Unnecessary parentheses here:

if (w == 145):

The augmented assignment operator +=

Instead of:

    intervals = intervals + (intervals % 2)

This is better:

    intervals += intervals % 2

Use one statement per line

Break the line after :, for example instead of:

for m in (n-1) * [intervals]: xs.append(xs[-1] - m)

Write like this:

for m in (n-1) * [intervals]:
    xs.append(xs[-1] - m)

Avoid code in the global namespace

Move code out of the global namespace into methods, so instead of this:

for n in range(2, 41):
    print("Version {:2d}: {}".format(n, pattern(n)))

Do it this way:

def main():
    for n in range(2, 41):
        print("Version {:2d}: {}".format(n, pattern(n)))

if __name__ == '__main__':
    main()


Building Better Boxes I'm working with parameters right now in Java, and my colleague told me that this code of mine is redundant, and I see his point. The for loops do look redundant, but I have no idea how to go about changing this. My code is functional, but I wish to improve it.

public static void printLine(int length)
{
    for (int i = 1; i <= length; i++)
    {
        System.out.print("*");
    }
    System.out.println();
}

public static void printBox(int length, int height)
{
    printLine(length);

    for (int i = 1; i <= height - 2; i++)
    {
    System.out.print("*");
        for (int j = 1; j <= length - 2; j++)
        {
            System.out.print(" ");
        }
        System.out.println("*");
    }
    printLine(length);
}

Your friend is right, and there's ways to parameterize the code to make it more useful. Consider three things:

Don't print each character - build a block of text up, and print the block (note that adding a newline to the string \n works as well as a System.out.println())
You can input the text to repeat as a parameter to the function...
It is common practice to have loops from 0 to 1-less-than-the-limit like for (int i = 0; i < limit; i++) {....} and not from 1 to the limit for (int i = 1; i <= limit; i++) {...}. Your loop is not wrong.... but it is unconventional.

So, take your function:

public static void printLine(int length)
{
    for (int i = 1; i <= length; i++)
    {
        System.out.print("*");
    }
    System.out.println();
}

And apply the three concepts above... and we get:

public static String repeatText(String text, int count) {
    StringBuilder output = new StringBuilder();
    for (int i = 0; i < count; i++) {
        output.append(text);
    }
    return output.toString();
}

Now, that function can be used as the basis for a bunch of things....

your top-and-bottom lines of the box.
the inside-content of the other lines of the box
the repeating rows inside the box

Consider your printBox function:

public static void printBox(int length, int height)
{
    printLine(length);

    for (int i = 1; i <= height - 2; i++)
    {
    System.out.print("*");
        for (int j = 1; j <= length - 2; j++)
        {
            System.out.print(" ");
        }
        System.out.println("*");
    }
    printLine(length);
}

Let's use the same 3 principles I listed earlier, but let's also use the new repeatText function....

public static String buildBox(int length, int height)  {
    String topAndBottom = repeatText("*", length) + "\n";
    String middle = "*" + repeatText(" ", length - 2) + "*\n";
    String box = topAndBottom + repeatText(middle, height - 2) + topAndBottom;
    return box;
}

Now, you can print that box just once with:

System.out.print(buildBox(length, height));

There are still issues, like what happens if the length or height are less than 2? That would be a problem. Consider ways to avoid that, or handle that.


Chinese Poker using classes I developed a Chinese Poker game for better understanding classes in Python. It is not finished yet, but the main part is.

The game mechanics:

There is 1 board for each player. Each board have 3 rows.
The first row can receive up to 3 cards, and the other 2, 5 cards each. A total of 13 cards in each board
First, each player receive 5 cards and then place then within the boards. Player1 place all his cards, and then Player2 place all his cards.
After that, each player receive 3 cards per turn. Place 2 cards in the boards, and discard the last one. Cards are dealed until the board is full.

My code executes them and the score system is the next step. I'd like you to evaluate my code and point suggestions to improve it.

from random import shuffle

def turn_str(n):
    return '%d' % n

deck =  (map(turn_str, range(2,11)) + ['J', 'Q', 'K', 'A']) * 4
deck.sort()

naipes = ['c', 'h', 's', 'd'] * 13

def add_suits(x, y):
    return x + y

full_deck = map(add_suits, deck, naipes)
shuffle(full_deck)

class User(object):
    def __init__ (self, name):
        self.name = name
        self.board = [[],[],[]]
        self.hand = []

    def receive_cards(self, number_of_cards):
        self.hand = full_deck[0:number_of_cards]
        del full_deck[0:number_of_cards]

    def board_limit(self, board_number):
        if board_number == 1:
            if len(self.board[board_number - 1]) < 3:
                return True
            else:
                return False
        if board_number == 2 or board_number == 3:
            if len(self.board[board_number - 1]) < 5:
                return True
            else:
                return False

    def place_first_cards(self):
        print "Your time to play %s. Here is your hand. Pick a card and choose a row to place it" % self.name
        while len(self.hand) != 0:
            print '-'.join(self.hand)
            for i in self.board:
                print i
            card_placed = int(raw_input("Choose a card (1 - %d): " % len(self.hand)))
            chosen_board = int(raw_input("Choose a board (1-3): "))
            if self.board_limit(chosen_board):
                self.board[chosen_board - 1].append(self.hand[card_placed - 1])
                self.hand.pop(card_placed - 1)
            else:
                print "The board is full. Choose another one."

    def place_cards(self):
        print "Your time to play %s. Here is your hand. Pick a card and choose a row to place it" % self.name
        while len(self.hand) != 1:
            print '-'.join(self.hand)
            for i in self.board:
                print i
            card_placed = int(raw_input("Choose a card (1 - %d): " % len(self.hand)))
            chosen_board = int(raw_input("Choose a board (1-3): "))
            if self.board_limit(chosen_board):
                self.board[chosen_board - 1].append(self.hand[card_placed - 1])
                self.hand.pop(card_placed - 1)
            else:
                print "The board is full. Choose another one."
        self.hand.pop(0)

player1 = raw_input("Player 1, type your nickname: ")
player2 = raw_input("Player 2, type your nickname: ")

user1 = User(player1)
user2 = User(player2)

print "Welcome. %s you will play against %s. Good luck for both of you." % (user1.name, user2.name)

user1.receive_cards(5)
user2.receive_cards(5)

user1.place_first_cards()
user2.place_first_cards()

user1.receive_cards(3)
user2.receive_cards(3)

user1.place_cards()
user2.place_cards()

user1.receive_cards(3)
user2.receive_cards(3)

user1.place_cards()
user2.place_cards()

user1.receive_cards(3)
user2.receive_cards(3)

user1.place_cards()
user2.place_cards()

user1.receive_cards(3)
user2.receive_cards(3)

print user1.board
print user2.board

Repeat actions

Look at the bottom of your code. This is the layout:

do something with user1
do the same thing with user2

do something with user1
do the same thing with user2

...

You have a lot of repeated code down there. To reduce this repetition, you should store the two players in an array, and then each time loop through the array an do what you need to do with each player.

To make it cleaner, you could use comprehensions:

user.receive_cards(5)    for user in users
user.place_first_cards() for user in users
user.receive_cards(3)    for user in users
...

This clearly illustrates each step of the game play, and doesn't have a repetitive line right after each one.

In fact, you could simplify this even further. If you keep a list of lambdas that show/do each of the different game moves (receive_cards, play_first_cards, etc.), then you easily loop through the list, calling each lambda.

Here's what the list might look like:

game_moves = [lambda user: user.receive_cards(5), lambda user: user.place_first_cards(), ...]

Then:

for game_move in game_moves:
    game_move(for user in users)

Note: I didn't have time to test this out, so this may not work. Leave a comment if there is an issue; I'll fix it later.

Now, with the list, each part of the game item by item is easy visible just a glance at the list.

If this, return true. Else that, return false.

I came across this in your code, inside your board_limit method:

if len(self.board[board_number - 1]) < 3:
    return True
else:
    return False

This is unnecessary. If the condition in the if statement evaluates to true, why don't you just return the conditional itself?

return len(self.board[board_number - 1]) < 3

Refactor methods

Look at your place_first_cards and place_cards methods. Then, put them in a diff tool. What's the different? Here:

while len(self.hand) != 0:

and:

self.hand.pop(0)

These are the only two lines that are different among these two methods. Everything else is exactly the same.

You should refactor this into one method that takes a parameter that tells if the cards are the first cards or not. Then, your new method would look like this:

def place_cards(self, is_first):
    print "Your time to play %s. Here is your hand. Pick a card and choose a row to place it" % self.name
    while len(self.hand) != (0 if is_first else 1): <--------
        print '-'.join(self.hand)
        for i in self.board:
            print i
        card_placed = int(raw_input("Choose a card (1 - %d): " % len(self.hand)))
        chosen_board = int(raw_input("Choose a board (1-3): "))
        if self.board_limit(chosen_board):
            self.board[chosen_board - 1].append(self.hand[card_placed - 1])
            self.hand.pop(card_placed - 1)
        else:
            print "The board is full. Choose another one."
    if not is_first:
        self.hand.pop(0) <------

This single method does exactly what those other two methods did, except for two small sections that can be "turned on or off" with a simple argument.

Now, your code would look like this for placing first cards:

user.place_cards(is_first=True)

Note: the is_first= is not necessary, but it increases readability, I think.

Now your code is simpler.


Implementing a Trie in Python - follow-up This is a follow-up of Wikipedia Trie pseudocode in Python

Code quality improvements

find has been fixed and a regression test ("bana" vs "banana") has been added so that it will never again be broken that way.
Used ordinary instance methods when sensible (previous code suffered from staticmethods overuse).
Significant simplification from the use of Python defaultdict
Operator overloading to provide easy membership test and printing (__repr__, __contains__)
Removed the value argument as I saw no use for it.
Minor doc-style language adjustments.
The printing is not used in testing anymore, as it should not be, because it is arbitrary (as Python's dicts are)

Functionality improvements

The printing is now as it should be, not reversed.
There is no more a weird inspect method, you can simply print the trie.

Example output

A trie from the words ('banning', 'banned', 'banana', 'bad', 'cooking', 'cought', 'count') is printed as:

c
 o
  o
   k
    i
     n
      g
       #
  u
   g
    h
     t
      #
   n
    t
     #
b
 a
  n
   a
    n
     a
      #
   n
    i
     n
      g
       #
    e
     d
      #
  d
   #

# signals the end of a word.

The code

import collections
import doctest

class Trie:
    """
    Implements a Trie (also known as 'digital tree',  'radix tree' or 'prefix tree').

    Where common starting letters of many words are stored just once.
    """
    def __init__(self):
        self.child = collections.defaultdict(Trie)

    def insert(self, string):
        """
        Add a given string to the trie, modifying it **in place**.

        >>> t = Trie()
        >>> t.insert('hello')
        >>> t.insert('hi')
        >>> list(sorted(t.child.keys()))
        ['h']
        >>> first_node = t.child['h']
        >>> list(sorted(first_node.child.keys()))
        ['e', 'i']

        As you can see, the same `h` was written only once,
        and it is connected with both `i` and `e`.
        """
        node = self
        for char in string:
            node = node.child[char]
        node = node.child[None]

    def __contains__(self, word):
        """
        >>> t = Trie()
        >>> t.insert('example')
        >>> 'example' in t
        True
        >>> 'exemplum' in t
        False
        >>> t.insert('bana')
        >>> 'banana' in t
        False
        >>> t.insert('banning')
        >>> t.insert('banned')
        """
        trie = self

        for char in word:
            if char in trie.child:
                trie = trie.child[char]
            else:
                return False

        return True

    def __str__(self, depth = 0):
        """
        Shows a nicely formatted and indented Trie.

        Cannot be tested as equivalent representations
        are arbitrarly chosen from (`dict`s are not ordered).
        """
        s = []
        for i in self.child:
            s.append( '{}{} {}'.format(
                ' ' * depth, i or '#', '\n' + self.child[i].__str__(depth + 1)))
        return ''.join(s)

if __name__ == '__main__':
    doctest.testmod()
    trie = Trie()
    for word in ('banning', 'banned', 'banana', 'bad', 'cooking', 'cought', 'count'):
        trie.insert(word)
    print(trie)

You could easily extent __init__ to take a list of strings to add to your Trie from the get go.

def __init__(self, *args):
    self.child = collections.defaultdict(Trie)
    for arg in args:
        self.insert(arg)

Now you could pass an arbitrary number of strings to Trie and they'll all be added in initialisation.

Also, if you wanted to you could still make the string representations equivalent by sorting the list before you return it.

def __str__(self, depth = 0):
    s = []
    for i in self.child:
        s.append( '{}{} {}'.format(
            ' ' * depth, i or '#', '\n' + self.child[i].__str__(depth + 1)))
    s.sort()
    return ''.join(s)

If you're concerned with running time, you can make it flaggable and off by default. Speaking of defaults, you should use depth=0 (no spaces, PEP8 reference) as that's the commonly accepted syntax.


Best way to iterate over JSON So this is my first time going over a pretty big nested object, was posting this up to see how I can possibly cut this down. (Some fears I have would be, I currently know the extent of the nesting (4) but could be more in the future. Which I'm not accounting for. )

The code below sits within a function:

if (itemLi.items) {

var ul = new Element('ul').inject(el)
for (var i = 0; i < itemLi.items.length; i++) {
    var li = new Element('li').inject(ul);
    lastPgNum = itemLi.items[i].page;
    //subItem = new Element('a', {'href': itemLi.items[i].href,'html': itemLi.items[i].title}).inject(li);
    if (itemLi.items[i].items) {

        subItem = new Element('span', {
            'html': itemLi.items[i].title,
            'class': 'pgTitle'
        }).inject(li);
        var ulsub = new Element('ul', {
            'class': 'here1'
        }).inject(ul)

        for (var b = 0; b < itemLi.items[i].items.length; b++) {
            var liSub = new Element('li').inject(ulsub);
            lastPgNum = itemLi.items[i].items[b].page;
            countSubSubs++;

            if (itemLi.items[i].items[b].items) {
                subItem = new Element('span', {
                    'html': itemLi.items[i].items[b].title,
                    'class': 'pgTitle'
                }).inject(liSub);
                var ulsubTwo = new Element('ul', {
                    'class': 'here2'
                }).inject(ulsub)
                for (var c = 0; c < itemLi.items[i].items[b].items.length; c++) {
                    var li = new Element('li').inject(ulsubTwo);
                    lastPgNum = itemLi.items[i].items[b].items[c].page;
                    countSubSubsSub++;
                    subSubItem = new Element('a', {
                        'href': itemLi.items[i].items[b].items[c].href,
                        'html': itemLi.items[i].items[b].items[c].title
                    }).inject(li);
                };
                var startPgSub = new Element('span', {
                    'class': 'pgNumbers',
                    'html': 'pg ' + (lastPgNum - (countSubSubs + countSubSubs + i + 1)) + '-' + lastPgNum
                }).inject(name);
            } else {

                var startPgSub = new Element('span', {
                    'class': 'pgNumbers',
                    'html': 'pg ' + (lastPgNum - (countSubSubs)) + '-' + lastPgNum
                }).inject(name);
                subItem = new Element('a', {
                    'href': itemLi.items[i].items[b].href,
                    'html': itemLi.items[i].items[b].title
                }).inject(liSub);
            }

        };
    } else {

        subItem = new Element('a', {
            'href': itemLi.items[i].href,
            'html': itemLi.items[i].title
        }).inject(li);

        if ((lastPgNum - i) == lastPgNum) {
            startPg = new Element('span', {
                'class': 'pgNumbers',
                'html': 'pg ' + lastPgNum
            }).inject(name);
        } else {
            startPg = new Element('span', {
                'class': 'pgNumbers',
                'html': 'pg ' + (lastPgNum - i) + '-' + lastPgNum
            }).inject(name);
        }

    }
};

};

You could use a recursive function to traverse your object and generate the HTML.

Something like:

function generateList(items, depth) {
    if (!depth) depth = 0;
    var ul = new Element('ul');
    if (depth) { ul.addClass('here' + depth); }
    for (var i = 0; i < items.length; ++i)
    {
        var item = items[i];
        var li = new Element('li').inject(ul);
        if (item.items) {
            li.adopt(new Element('span', {
                'html' : item.title,
                'class': 'pgTitle'
            }));
            li.adopt(generateList(item.items, depth + 1));
        } else {
            li.adopt(new Element('a', {
                'href' : item.href,
                'title': item.title
            }));
        }
    }
    return ul;
}

if (itemLi.items) {
    generateList(itemLi.items).inject(el);
}

This doesn't include code to add the page spans, cause i don't understand yet how they should be generated.  (The code doesn't include declarations of the variables involved, for starters.)  But it should generate a list of links, with sublists for any items that have sub-items.  The classes for the sublists are kinda unnecessary -- CSS can select them just fine as ul>li>ul and ul>li>ul>li>ul -- so unless you have some other need to track the amount of nesting, you could get rid of depth and the class on the uls.


32-bit counter and test bench How could this VHDL counter and its test bench be improved?  I am interested in anything you see that could be done better, but especially in the test bench:

Is wait for 10 ns better or worse than any other time delay?
The test is very minimal.  Should it do more?

counter32.vhdl:

library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;

entity counter32 is
  port (
    clk: in std_ulogic;
    ena: in std_ulogic;
    rst: in std_ulogic;
    q  : out std_ulogic_vector(31 downto 0));
end entity;

architecture rtl of counter32 is

  signal count : unsigned(31 downto 0);

begin

  process(clk, rst)
  begin
    if rst = '1' then
      count <= X"00000000";
    elsif rising_edge(clk) then
      if ena = '1' then
        count <= count + 1;
      end if;
    end if;
  end process;

  q <= std_ulogic_vector(count);

end architecture;

counter32_tb.vhdl:

library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;

entity counter32_tb is
end counter32_tb;

architecture tb of counter32_tb is

  component counter32
    port (
      clk: in std_ulogic;
      ena: in std_ulogic;
      rst: in std_ulogic;
      q  : out std_ulogic_vector(31 downto 0));
  end component;

  signal clk: std_ulogic;
  signal ena: std_ulogic;
  signal rst: std_ulogic;
  signal q  : std_ulogic_vector(31 downto 0);

begin

  dut : counter32 port map (
    clk => clk,
    ena => ena,
    rst => rst,
    q => q);

  process
  begin

    -- Reset
    clk <= '0';
    ena <= '1';
    rst <= '1';
    wait for 10 ns;
    rst <= '0';
    wait for 10 ns;

    -- Counts on clock leading edge if enabled
    clk <= '1';
    wait for 10 ns;
    assert q = x"00000001";

    -- Does not count on clock trailing edge
    clk <= '0';
    wait for 10 ns;
    assert q = x"00000001";

    -- Does not count when not enabled
    ena <= '0';
    clk <= '1';
    wait for 10 ns;
    assert q = x"00000001";

    -- Clears on reset
    rst <= '1';
    wait for 10 ns;
    assert q = x"00000000";

    wait;

  end process;

end tb;

Shell script to run the test bench in GHDL:

OPTS=--workdir=build
ghdl -a ${OPTS} *.vhdl
ghdl -e ${OPTS} counter32_tb
ghdl -r ${OPTS} counter32_tb --vcd=counter32.vcd

Nice with some HDL on Code Review. I'll contribute with my opinions. All-in-all, I find the code all well-written.
Opinions not already covered in previous answers

A more standard way of setting count at reset would be (others => '0').

That is, as opposed to hard-coding a bit/hex literal - even if you for some reason hard-code the width as you have done, (others => '0') is more standard in this case.

You might have a good reason for asynchronous reset, but if not, synchronous reset is a better habit.

You often see these asynchronous resets, it is unfortunately a more common habit, since that's what is (or was) most often taught.
See my motivations here: http://www.fpga-dev.com/resets-make-them-synchronous-and-local/

Opinions already covered/partly covered
Somewhat in order of importance (in my subjective opinion).

Define the roll-over behaviour.

The behaviour is actually defined for an unsigned in numeric_std: any carry bits will be ignored so the unsigned will roll-over. However, neither simulators nor synthesis tools should be trusted on following this. (And even less, humans reading the code.) Also; it is good to really think about what behaviour you desire, or is the most stable: roll-over or saturate? I'll take the chance to give two comments on this choice:

For synthesis, roll-over rather than saturation will yield less logic and improved timing. The tool will be able to use a counter macro right-off.
If reaching max count is expected never to happen, add an assertion so a simulation will flag in that case. Then still code the RTL behaviour explicitly to roll-over (less logic, better timing) if there aren't stability reasons to code it to saturate.

Parameterize the width. Put it in a constant or generic.

Generate the clock separately (e.g. clk <= not(clk) after 10 ns;, and make sure to init clk to '1'.

This is more standard and more readable.
Then, when coding your stimuli, do not dead-reckon the time for stimuli assignments, instead, use wait until rising_edge(clk); etc. This makes code more stable, and gives deterministic and proper times for the stimuli changes (not only in regards to time, but also to simulation deltas, which is equally important).

Use entity instantiation

More details and motivation: http://www.fpga-dev.com/leaner-vhdl-with-entity-instantiation/

Simulation performance

There's nothing wrong in coding the RTL of the counter like this, but it could be noted that there is a simulation performance gain possible: Make count a variable, local to the process, do tests and arithmetics on this variable, and in the end of the process drive the variable to a counter signal (defined in the architecture).

Others

Thumbs up for self-checking test benches.


Read CSV files and find the same values on the specific columns I created a simple command line application to read CSV files in a specific folder and find the same values on the specify column which exists on all files. (Here's the complete source code.)

Here's the core of the logic:

func csvReader(inputColumn, fileLocation string) {
    startTime := time.Now()

    // get all the files from the specific folder
    files, err := ioutil.ReadDir(fileLocation)
    if err != nil {
        log.Fatal(err)
    }

    noColumn := make(chan struct{}, len(files))
    results := make(chan map[string]bool, len(files))

    // reading all the files from dir concurrently
    for _, file := range files {
        wg.Add(1)
        // and read it concurrently to get the data from specific column
        go func(file os.FileInfo) {
            defer wg.Done()
            f, err := os.Open(dir + file.Name())
            if err != nil {
                log.Fatal(err)
            }
            datas, ok := readFile(inputColumn, f)

            if !ok {
                noColumn <- struct{}{}
                return
            }

            results <- datas

        }(file)

    }

    wg.Wait()

    // check if we got the column or not
    select {
    case <-noColumn:
        fmt.Printf("The column name = %v doesnt exist\n", inputColumn)
        return

    default: // do nothing and continue
    }

    close(results)
    close(noColumn)

    //receive results and determine which size datas is the smallest
    theSameValue := getSameValues(&results)

    fmt.Printf("final result = %+v\n", theSameValue)
    fmt.Printf("final result size = %+v\n", len(theSameValue))
    fmt.Printf("time consume = %+v\n", time.Since(startTime).Seconds())

}

The function is finding the files on specified fileLocation and then concurrently reading the file to get the values from the given column inputColumn, and passing the values to channel to save the results from reading the CSV files.

The channel results := make(chan []map[string]bool) is passed to a function the begin searching the values that exist on all files.

theSameValue := getSameValues(&results)

Notice that the function getSameValues() receives the pointer of results. I did this because I don't' want the results channel to be copied to the functions argument which takes resource of memory.

And here is the getSameValues() function:

// getting the same value from all the datas
func getSameValues(results *chan map[string]bool) []string {
    var datas = make([]map[string]bool, len(*results))
    minIndex := -1
    minSize := int(MaxUint >> 1)
    i := 0
    for values := range *results {
        sizeValues := len(values)
        if sizeValues < minSize && sizeValues > 0 {
            minSize = sizeValues
            minIndex = i
        }
        datas[i] = values
        i++
    }

    // getting the same value from all the datas
    var theSameValue []string
    for value, _ := range datas[minIndex] {
        isExistAll := false
        for _, data := range datas {
            if data[value] {
                isExistAll = true
            } else {
                isExistAll = false
            }
        }

        if isExistAll {
            theSameValue = append(theSameValue, value)
        }
    }

    return theSameValue
}

Hopefully this would help others and if you find something that can be improved please give some suggestion.

Pointers to chan

Channel variables are references to an underlying data structure.
As such, in terms of resources, I don't think you gain anything at all by passing a *chan instead of the chan itself.
I re-read the section on channels in Effective Go,
and there's no mention about saving resources by passing around pointers,
and it's not logical.
So I think you can safely stop using a pointer for this and simplify the code.

Likely bug

This can be simplified:

isExistAll := false
for _, data := range datas {
    if data[value] {
        isExistAll = true
    } else {
        isExistAll = false
    }
}

You can use the value of data[value] directly:

isExistAll := false
for _, data := range datas {
    isExistAll = data[value]
}

But I don't think this code does what you intended.
The value of isExistAll will be the value of the last data[value].
I think you want it to be true only of all values are true:

isExistAll := true
for _, data := range datas {
    isExistAll = isExistAll && data[value]
}

Constructing paths

Instead of concatenating strings like dir + file.Name() to create paths,
it's better to use path.Join(dir, file.Name()).