Applying different equations to a Pandas DataFrame I wrote a task using pandas and I'm wondering if the code can be optimized. The code pretty much goes as:

All the dataframes are 919 * 919. socio is a dataframe with many fields. matrices is a dictionary that holds different dataframes objects.

I apply a series of formulas and store the results in the result dataframe.

import numpy as np

frame = [zone for zone in range(taz_start_id, taz_end_id +1)]
#frame = list from 1 to 919

result = DataFrame(0.0, index=frame, columns=frame)

#Get a seires type and apply the results to all the rows of the dataframe by index
temp = np.log(socio["ser_emp"] + 1) * 0.36310718
result = result.apply(lambda x:  x + temp[x.index], axis = 0)

#Divide two columns apply a coefficient and fill all the nan to 0. apply results to result dataframe
temp = (socio["hhpop"] / socio["acres"]) * -0.07379568
temp = temp.fillna(0)
result = result.apply(lambda x:  x + temp[x.index], axis = 0)

result = (matrices['avgtt'].transpose() * -0.05689183) + result

# set a 1.5 value if dist is between values or 0 if not
result =((matrices["dist"] > 1) & (matrices["dist"] <= 2.5))  * 1.5 + result

# see if each cell is 0 if not set value to exp(value)
result = result.applymap(lambda x: 0 if x == 0 else exp(x))

You don't want to use an awkward lambda to do the addition.  You can use the add method with a parameter axis=0 that specifies which axis to match the lower dimensional argument.

I bundled the two Series you are adding into one so you only have to broadcast across the index once.

Next, I just changed to match my style and improve my readability

Finally, the finding where not zero and filling in with exp, I'm assuming exp is numpy's exp via some import from numpy import exp or something.  Otherwise, replace with np.exp.  This is much more vectorized.  result is a dataframe and mask makes anything that evaluates to True from the passed condition statement into np.nan. I then chain a fillna with a vectorized call to np.exp(result)

result = result.add(
    np.log(socio.ser_emp.add(1)).mul(.36310718).add(
        socio.hhpop.div(socio.acres).mul(-.07379568).fillna(0)
    ), axis=0)

avgtt, dist = matrices['avgtt'], matrices['dist']
result += avgtt.T * -0.05689183 + ((dist > 1) & (dist <= 2.5)) * 1.5

result = result.mask(result != 0).fillna(exp(result))


Custom memcmp function I have written my own memcmp() function for learning purposes:

bool memcmp( byte* _bufA, byte* _bufB, uint n )
{
    while ( n > 4 )
    {
        if ( (int)*_bufA != (int)*_bufB )
            return false;
        _bufA += 4, _bufB += 4;
        n -= 4;;
    }
    while( --n )
    {
        if ( *_bufA != *_bufB )
            return false;
        ++_bufA, ++_bufB;
    }
    return true;
}

My thought process was that whenever the byte stream is greater than 4 bytes, I'd compare 4 bytes at a time instead of one. That means fewer iterations which would mean it would be a little more faster. And then just simply take care of the remaining 4 bytes or so in the second while statement.

Benchmarks:

Standard memcmp function comparing 1024 bytes: 585 nanoseconds
My memcmp function comparing 1024 bytes: 586 nanoseconds

It's pretty inconsistent but I figure it's still a pretty good function since it's 50/50 generally.

Correctness

Your function does not work correctly. In my experience, memory manipulating functions almost never work correctly as they are written the first time. (At least when I'm writing them.) If you need to go down into that rabbit hole, be sure to do so with an extensive set of unit-tests. In your case, you are lucky. You have two functions you can compare your results with:

The existing memcmp function from the standard library and
the naïve implementation of a for loop that compares one byte after the other.

When I'm writing optimized functions, I usually start with the naïve implementation. Then I write a comprehensive set of tests for it to make sure I got it right. (For the naïve version, this is usually easy.) Then I start optimizing the function and after each change, run the test suite again. Should one of my optimizations introduce a correctness bug, I'll notice.

Also be sure to run your unit tests trough Valgrind and compile test programs with sanitizers added. For GCC and Clang, all you have to do is pass the -fsanitize=WHAT switch where WHAT is one of address, memory or undefined. (memory is only supported by Clang yet.)

Cast the pointer, not the value

Have a look at this piece.

if ((int) *_bufA != (int) *_bufB)
  return false;

What does it do? It dereferences the pointers _bufA and _bufB which are of type byte * (which I assume is an alias for char *). Now, only after that, you cast the result (which is of type byte) to an int. In effect, you're only looking at each forth byte.

What you have to do instead is casting the pointer before dereferencing it.

if (*((int *) _bufA) != *((int *) _bufB))
  return false;

Watch out for off-by-one errors

Your second loop is off-by-one.

while (--n) { … }

If I call your function with n == 1, it'll never look at any byte. If chux decides to call it with n == 0, it will explode. The correct version would be to use post-decrement.

while (n--) { … }

On the other hand, your first loop could use

while (n >= 4) { … }

instead of

while (n > 4) { … }

even though this “bug” doesn't cause the function to produce a wrong answer, it is probably not what you've meant.

Do memory operations on unsigned types

While I don't think it would cause a problem in this particular case, using signed integer types can lead to all kinds of surprises. If you don't treat the values as numbers but as a bunch of bytes, stay with unsigned types.

Beware of undefined behavior

The C standard does not allow you to cast a pointer to another type (except char) to access the memory it points to. (This is a simplified explanation, search for “strict aliasing rules” for more information.) Therefore, your function invokes undefined behavior and this cannot be fixed other than by using the standard library functions.

That said, I think that this is a case where you can get pretty far with (extremely) carefully blundering the line of undefined behavior if you know what you are doing.

On many machines, memory must be aligned. That is, an object o of type T must have an address such that ((uintptr_t) &o) % alignof(T) == 0 where alignof(T) is called the alignment of type T. Unfortunately, alignof is not an operator in C (unlike in C++ since C++11). A safe estimate is to use the maximum of sizeof(T) and 8. Even on those machines where an unaligned memory access does not cause an exception, it might be slow.

Therefore, what you should do is check whether both pointers can be word-aligned by advancing both of them by the same number of bytes. If so, first do a short loop that aligns the buffers, then cast to a machine word type and compare the bulk. Finally, compare the remaining bytes with a byte-for-byte loop again.

-fsanitize=undefined can warn you about mis-aligned reads.

Portability

Don't make assumptions on sizeof(int)

Your function silently assumes that sizeof(int) == 4. This will not be true on all platforms. Instead of hard-coding such values, use sizeof(int). You can store it in a constant if repeating it seems too much typing.

Performance

Prefer longs over ints

In absence of any reason to do otherwise, unsigned long is your best bet to get a type that directly maps to a machine-word and will therefore give you optimal performance. Once I made this change to your code, I got significant speedup on my 64 bit machine from 2.9 GiB/s to 4.3 GiB/s!

Consider exiting early

If you anticipate that your function might regularly be called with aliasing pointers, it might be worthwhile to check for this.

if (p1 == p2)
  return true;

It doesn't cost much if the condition is false and will be an enormous short-cut if the pointers do alias.

Be sure to measure

Doing low-level optimization without measuring is a waste of time. You should compare your function with (at least) two alternatives:

the naïve byte-for-byte approach and
at least one highly optimized standard library implementation.

Before you do any optimization, be sure to compare those two approaches when compiled with all compiler optimizations turned on. If they don't differ much, you're probably done. Otherwise, deploy one optimization at a time and see how the performance of your implementation goes from “naïve” to “standard library”. If it goes below “naïve”, you're doing something wrong. If it goes above “standard library”, you did something awesome (or your function has a bug and doesn't do the same thing as the standard library version).

Writing good benchmarks is very hard. As a general rule, don't look at a single input but measure how the run-time scales with the input size. Also investigate the inevitable statistical errors your measurements will contain. I like to fit regressions to the data series and get a statistically more significant result from the fit parameters. Be sure to test large enough inputs so to reduce noise. Timings in the sub-millisecond range as you reported them are hardly significant.

I did this for your problem and got this result on my machine. Your version (with the fixes discussed above and using unsigned long) is not doing bad. Note, however, that the benchmark always tests the lucky case where the buffers can be aligned. I'll leave it as an exercise to you to test the more realistic case, too.

I'm dumping the code I've used to produce this data without any explanation here only so you can repeat the experiments on your hardware if you want to. It is a quick-and-dirty solution and I'm not particularly proud of the coding style but I believe that it tests accurately.

main.c:

#define _XOPEN_SOURCE 500  // random, srandom

#include <assert.h>        // assert
#include <math.h>          // isfinite, sqrt
#include <stdbool.h>       // bool, false, true
#include <stddef.h>        // size_t
#include <stdio.h>         // FILE, fopen, fclose, fprintf, snprintf, stderr
#include <stdlib.h>        // NULL, EXIT_{SUCCESS,ERROR}, malloc, free, random, srandom
#include <string.h>        // memcpy
#include <time.h>          // CLOCKS_PER_SEC, clock_t, clock

#include "memcmp.h"

// Tells the compiler that all previous changes to memory shall be visible.
#define CLOBBER_MEMORY()   __asm__ volatile ( "" : : : "memory" )

// Tells the compiler that the value of VAR shall be visible.
#define USE_VARIABLE(VAR)  __asm__ volatile ( "" : : "rm"(VAR) : )

// Type of the function to benchmark.
typedef bool (* memcmp_type)(const void *, const void *, size_t);

// Unsigned integer type that refers to a machine word.
typedef unsigned long word_type;

// Size of a machine word.
static const size_t word_size = sizeof(word_type);

// Global error message.
static const char * errmsg;

// Average value and statistical error of a statistical data sample.
typedef struct
{
  double average;  // average value (eg mean)
  double error;    // statistical error
} timing_result;

// Computes the square of a number.
static double
square(const double x)
{
  return x * x;
}

// Computes mean and standard deviation of the `n` data points in the array
// `datapoints`.  `n` must be at least 3.  The mean is stored at
// `resultptr->average` and the standard deviation at `resultptr->error`.
// Returns 0 on success.  On error a negative number is returned and `errmsg`
// is set.
static int
do_statistics(const double *const datapoints,
              const size_t n,
              timing_result *const resultptr)
{
  assert(n >= 3);
  double accu = 0.0;
  for (size_t i = 0; i < n; ++i)
    accu += datapoints[i];
  const double mean = accu / n;
  if (!isfinite(mean))
    {
      errmsg = "non-finite mean";
      return -1;
    }
  accu = 0.0;
  for (size_t i = 0; i < n; ++i)
    accu += square(datapoints[i] - mean);
  const double stdev = sqrt(accu / (n - 1));
  if (!isfinite(stdev))
    {
      errmsg = "non-finite standard deviation";
      return -1;
    }
  resultptr->average = mean;
  resultptr->error = stdev;
  return 0;
}

// Randomly maybe changes a single byte in the word pointed to by `wp`.
// Returns whether any byte was changed.
bool
maybe_change_byte(word_type *const wp)
{
  assert(wp != NULL);
  unsigned char bytes[word_size];
  const word_type before = *wp;
  memcpy(bytes, wp, word_size);
  const size_t index = random() % word_size;
  if ((random() % word_size) != 0)
    bytes[index] = (unsigned char) random();
  memcpy(wp, bytes, word_size);
  return (*wp != before);
}

// Runs a single benchmark for the function `funcptr` on an input of
// approximately `input_size` bytes.  The function will be called one more than
// `repetitions` times and the execution time of all but the first run will be
// collected statistically and stored in the record pointed to by `resultptr`.
// `repetitions` must be at least 3.  `buff1st` and `buff2nd` must be buffers
// of at least `input_size` bytes that can be accessed as arrays of
// `word_type`.  The buffers must not overlap.  They will be used internally as
// work-space.  On success, 0 is returned.  On error, a negative number is
// returned and `errmsg` is set.
static int
run_benchmark(const memcmp_type funcptr,
              const size_t input_size,
              const size_t repetitions,
              word_type *restrict const buff1st,
              word_type *restrict const buff2nd,
              timing_result *const resultptr)
{
  assert(funcptr != NULL);
  assert(repetitions >= 3);
  assert(buff1st != NULL);
  assert(buff2nd != NULL);
  assert(buff1st != buff2nd);
  assert(resultptr != NULL);
  const size_t words = input_size / word_size;
  const size_t bytes = words * word_size;
  double timings[repetitions + 1];
  for (size_t i = 0; i < words; ++i)
    {
      const word_type w = random();
      buff1st[i] = w;
      buff2nd[i] = w;
    }
  for (size_t i = 0; i <= repetitions; ++i)
    {
      const clock_t no_clock = (clock_t) -1;
      const bool expected = (words >= 2)
        ? !maybe_change_byte(buff2nd + words - 2)
        : true;
      const size_t skip = (words >= 1) ? (random() % word_size) : 0;
      CLOBBER_MEMORY();
      const clock_t t0 = clock();
      const int actual = funcptr((const char *) buff1st + skip,
                                 (const char *) buff2nd + skip,
                                 bytes - skip);
      USE_VARIABLE(actual);
      const clock_t t1 = clock();
      if (actual != expected)
        {
          errmsg = "function returned wrong result";
          goto label_catch;
        }
      if ((t0 == no_clock) || (t1 == no_clock))
        {
          errmsg = "cannot get CPU time";
          goto label_catch;
        }
      timings[i] = (double) (t1 - t0) / CLOCKS_PER_SEC;
      if (words >= 2)
        buff2nd[words - 2] = buff1st[words - 2];
    }
  if (do_statistics(timings + 1, repetitions, resultptr) < 0)
    goto label_catch;
  resultptr->error += 1.0 / CLOCKS_PER_SEC;  // systematic error
  goto label_finally;
 label_catch:
  assert(errmsg != NULL);
 label_finally:
  return (errmsg == NULL) ? 0 : -1;
}

// Runs all benchmarks and for randomly chosen input sizes and writes the
// results to text files named `timing_${tag}.dat` in the current working
// directory where ${tag} identifies the benchmarked function.  Progress
// information is printed to standard error output.  Returns 0 on success.  On
// error, a negative number is returned and `errmsg` is set.
static int
run_all_benchmarks()
{
  const size_t repetitions = 10;
  const size_t datapoints = 50;
  const size_t max_size = 99 * (1ULL << 20);  // \lessapprox 100 MiB
  const size_t candidates = 3;
  const memcmp_type cand_funcs[] = {memcmp_naive, memcmp_stdlib, memcmp_yamiez};
  const char *const cand_names[] = {"naive", "stdlib", "yamiez"};
  assert(candidates == sizeof(cand_funcs) / sizeof(cand_funcs[0]));
  assert(candidates == sizeof(cand_names) / sizeof(cand_names[0]));
  word_type *const buff1st = malloc(max_size);
  word_type *const buff2nd = malloc(max_size);
  if ((buff1st == NULL) || (buff2nd == NULL))
    {
      errmsg = "out of memory";
      goto label_catch_outer;
    }
  for (size_t candidx = 0; candidx < candidates; ++candidx)
    {
      FILE * fh = NULL;
      fprintf(stderr, "%-12s ", cand_names[candidx]);
      char filename[128];
      const int status = snprintf(filename, sizeof(filename), "timing_%s.dat",
                                  cand_names[candidx]);
      if ((status < 0) || ((size_t) status >= sizeof(filename)))
        {
          errmsg = "error in snprintf";
          goto label_catch_inner;
        }
      fh = fopen(filename, "w");
      if (fh == NULL)
        {
          errmsg = "cannot open output file";
          goto label_catch_inner;
        }
      if (fprintf(fh, "# %22s %24s %24s\n\n", "n", "average / s", "error / s") < 0)
        {
          errmsg = "I/O error";
          goto label_catch_inner;
        }
      for (size_t j = 0; j < datapoints; ++j)
        {
          size_t n = random() % max_size;
          timing_result result = {0.0, 0.0};
          if (run_benchmark(cand_funcs[candidx], n, repetitions, buff1st, buff2nd, &result) < 0)
            goto label_catch_inner;
          if (fprintf(fh, "%24zu %24.10e %24.10e\n", n, result.average, result.error) < 0)
            {
              errmsg = "I/O error";
              goto label_catch_inner;
            }
          fputc('.', stderr);
        }
      goto label_finally_inner;
    label_catch_inner:
      assert(errmsg != NULL);
    label_finally_inner:
      if (fh != NULL)
        {
          if (fclose(fh) != 0)
            errmsg = "error closing output file";
        }
      fprintf(stderr, "  %s\n", (errmsg != NULL) ? "failed" : "done");
      if (errmsg != NULL)
        goto label_finally_outer;
    }
  goto label_finally_outer;
 label_catch_outer:
  assert(errmsg != NULL);
 label_finally_outer:
  free(buff1st);
  free(buff2nd);
  return (errmsg != NULL) ? -1 : 0;
}

// Seeds the global pseudo random number generator via `srandom` and runs all
// benchmarks.  Progress information and error messages are printed to standard
// error output.  Benchmark results are written to text files in the current
// working directory.  Returns `EXIT_SUCCESS` on success or `EXIT_FAILURE` on
// error.
int
main()
{
  srandom((unsigned int) clock());
  if (run_all_benchmarks() < 0)
    {
      if (errmsg == NULL)
        errmsg = "unknown error";
      fprintf(stderr, "error: %s\n", errmsg);
      return EXIT_FAILURE;
    }
  return EXIT_SUCCESS;
}

memcmp.h:

#ifndef MEMCMP_H
#define MEMCMP_H

#include <stdbool.h>  // bool
#include <stddef.h>   // size_t

#ifdef __cplusplus
extern "C" {
#endif

bool
memcmp_stdlib(const void * s1, const void * s2, size_t n)
  __attribute__ ((hot));

bool
memcmp_naive(const void * s1, const void * s2, size_t n)
  __attribute__ ((hot));

bool
memcmp_yamiez(const void * s1, const void * s2, size_t n)
  __attribute__ ((hot));

#ifdef __cplusplus
}  // extern "C"
#endif

#endif  // #ifndef MEMCMP_H

memcmp_naive.c:

#include "memcmp.h"

bool
memcmp_naive(const void *const s1, const void *const s2, const size_t n)
{
  const unsigned char *const c1 = s1;
  const unsigned char *const c2 = s2;
  for (size_t i = 0; i < n; ++i)
    {
      if (c1[i] != c2[i])
        return false;
    }
  return true;
}

memcmp_stdlib.c:

#include "memcmp.h"

#include <string.h>  // memcmp

bool
memcmp_stdlib(const void *const s1, const void *const s2, const size_t n)
{
  return memcmp(s1, s2, n) == 0;
}

memcmp_yamiez.c:

#include "memcmp.h"

#include <stdint.h>  // uintptr_t

bool
memcmp_yamiez(const void *const s1, const void *const s2, size_t n)
{
  typedef unsigned char byte_type;
  typedef unsigned long word_type;
  const size_t word_size = sizeof(word_type);
  const size_t word_align = (word_size >= 8) ? word_size : 8;
  const uintptr_t align_mask = word_align - 1;
  const byte_type * buf1 = s1;
  const byte_type * buf2 = s2;
  const uintptr_t addr1 = (uintptr_t) s1;
  const uintptr_t addr2 = (uintptr_t) s2;
  if ((addr1 & align_mask) == (addr2 & align_mask))
    {
      const uintptr_t skip = word_align - (addr1 & align_mask);
      for (uintptr_t i = 0; i < skip; ++i)
        {
          if (*buf1++ != *buf2++)
            return false;
          --n;
        }
      const word_type * wbuf1 = (const word_type *) buf1;
      const word_type * wbuf2 = (const word_type *) buf2;
      while (n >= word_size)
        {
          if (*wbuf1++ != *wbuf2++)
            return false;
          n -= word_size;
        }
      buf1 = (const byte_type *) wbuf1;
      buf2 = (const byte_type *) wbuf2;
    }
  while (n--)
    {
      if (*buf1++ != *buf2++)
        return false;
    }
  return true;
}

Here is the Makefile I have used to compile the benchmarks.

CC = gcc
CPPFLAGS = -DNDEBUG
CFLAGS = -std=c11 -Wall -Wextra -Werror -pedantic -O3
LDFLAGS =
LIBS = -lm

compile: main

all: plot.png

plot.png: plot.gp timing_stdlib.dat timing_naive.dat timing_yamiez.dat
    gnuplot $<

timing_stdlib.dat timing_naive.dat timing_yamiez.dat: main
    ./$<

main: main.o memcmp_stdlib.o memcmp_yamiez.o memcmp_naive.o
    ${CC} -o $@ ${CFLAGS} $^ ${LDFLAGS} ${LIBS}

main.o: main.c memcmp.h
    ${CC} -c ${CPPFLAGS} ${CFLAGS} $<

memcmp_stdlib.o: memcmp_stdlib.c memcmp.h
    ${CC} -c ${CPPFLAGS} ${CFLAGS} $<

memcmp_yamiez.o: memcmp_yamiez.c memcmp.h
    ${CC} -c ${CPPFLAGS} ${CFLAGS} $<

memcmp_naive.o: memcmp_naive.c memcmp.h
    ${CC} -c ${CPPFLAGS} ${CFLAGS} $<

mostlyclean:
    rm -f fit.log *.o

clean: mostlyclean
    rm -f main *.dat plot.png

.PHONY: compile all mostlyclean clean

And here is the Gnuplot script plot.gp that produces the graphic shown above.

#! /usr/bin/gnuplot

## If you set the environment variable INTERACTIVE to a positive integer, the
## script will show the plot in an interactive Gnuplot window instead of
## writing to a file 'plot.png'.

interactive = 0 + system('echo "${INTERACTIVE-0}"')

if (!interactive) {
   set terminal png size 800,600
   set output 'plot.png'
}

set key top left
set xlabel 'size / MiB'
set ylabel 'time / ms'

set xrange [0 : *]
set yrange [0 : *]

set format y '%.0f'
set ytics add ('0' 0)

## Colors are taken from the Tango palette.
## See http://tango.freedesktop.org/Tango_Icon_Theme_Guidelines .

set style line 1 lt 2 lw 2.0 ps 1.0 linecolor '#3465a4'
set style line 2 lt 1 lw 2.0 ps 1.0 linecolor '#cc0000'
set style line 3 lt 3 lw 2.0 ps 1.0 linecolor '#73d216'

## Prefix constants

kilo = 1000.0
mebi = 1048576.0

## Converts a regression constant 'c1' in milliseconds per byte to a throughput
## in gibibytes per second.

c2gibps(c1) = kilo / (c1 * 1024)

## Computes the maximum of two numbers.

max(x, y) = (x < y) ? y : x

## Converts a size in bytes to a size in mebibytes.

b2mib(n) = n / mebi

## Converts a time in seconds to a time in milliseconds.

s2ms(t) = kilo * t

regression_naive(x)  = c1_naive  * x
regression_stdlib(x) = c1_stdlib * x
regression_yamiez(x) = c1_yamiez * x

fit regression_naive(x)                                                       \
    'timing_naive.dat'  using (b2mib($1)):(s2ms($2)):(s2ms($3))               \
    yerror via c1_naive

fit regression_stdlib(x)                                                      \
    'timing_stdlib.dat' using (b2mib($1)):(s2ms($2)):(s2ms($3))               \
    yerror via c1_stdlib

fit regression_yamiez(x)                                                      \
    'timing_yamiez.dat' using (b2mib($1)):(s2ms($2)):(s2ms($3))               \
    yerror via c1_yamiez

plot 'timing_naive.dat' using (b2mib($1)):(s2ms($2)):(s2ms($3))               \
         with yerror ls 1                                                     \
         title sprintf('%s (%.2f GiB/s)', "naive", c2gibps(c1_naive)),        \
     regression_naive(x) notitle ls 1,                                        \
     'timing_stdlib.dat' using (b2mib($1)):(s2ms($2)):(s2ms($3))              \
         with yerror ls 2                                                     \
         title sprintf('%s (%.2f GiB/s)', "stdlib", c2gibps(c1_stdlib)),      \
     regression_stdlib(x) ls 2 notitle,                                       \
     'timing_yamiez.dat' using (b2mib($1)):(s2ms($2)):(s2ms($3))              \
         with yerror ls 3                                                     \
         title sprintf('%s (%.2f GiB/s)', "yamiez", c2gibps(c1_yamiez)),      \
     regression_yamiez(x) notitle ls 3

if (interactive) {
   print "Hit RET to quit ..."
   pause -1
}

Learn from the giants

There has been massive interest in optimizing memory functions and standard libraries have evolved highly optimized versions. If you want to learn from them, have a look at their code. For example, look at the version in the GNU C library. If reading that code gives you a headache, you are in good companion.

Coding Style

Use the proper types

In C, a read-only buffer of arbitrary data should be passed as const void *. The size of a buffer should be passed as size_t.

Be const correct

Since your function only ever reads the data from the buffer it is passed, it should be declared const.

Don't use identifiers that begin with an underscore

Identifiers that begin with an underscore followed by an upper-case letter and identifiers containing two adjacent underscores are reserved for the implementation. I recognize that your variables don't match this pattern but _bufA still looks bogus. Just drop the underscore.

Avoid confusing notation

I find this hard to read.

if (*bufA != *bufB)
  return false;
++bufA, ++bufB;

How about the commonly used form?

if (*bufA++ != *bufB++)
  return false;

Less typing and easier to understand. In general, I'd avoid putting more than one statement on a single line and not chain multiple statements together with the comma operator.


need to refactor multiple toggle lists $(document).ready(function() {

  $('#state-list, #major-list').hide();

  $('#state').bind('click', function() {
    $('#state-list').toggle();
  });

  $('#major').bind('click', function() {
    $('#major-list').toggle();
  });

});

All links and lists will follow the naming convention of:

link, link-list, like seen below:

Link

...

Is there a way listen for all links and toggle their respective lists, instead of creating a bind click for every link?

Link

 $('.listsToToggle').hide();

 $('.linksToWatch').bind('click', function() {
   var id = $(this).attr('id');
   $('#' + id + '-list').toggle();
 });

May be, something like this?


Systems management using SSH for my school I wrote this script in order to generate a hosts.txt file with the IP addresses of the laptops in my school.

Each laptop is assigned a number, and their IP addresses are assigned to them with the template "192.168.0.x". X being the computer's number + 10. All these computers are running Arch Linux with XFCE4, custom tailored to aid in education. Each laptop is setup with SSH and the Teacher, (me), has the ability to ssh into any machine and execute any command I deem necessary.

Up until recently, I would SSH into the machines individually and install programs one-by-one. Now I use a program called PSSH (Parallel SSH) to connect to certain machines simultaneously and perform my commands. I only find one issue with this arrangement, all the IP addresses are stored in a file called, you guessed it, hosts.txt.

Now, as you can imagine, this gets quite cumbersome when dealing with 70 different addresses, and I would rather not have to type all the addresses by hand. So I made this script to generate a hosts file based off the computer numbers I choose. Selecting all the machines is now as easy as typing 'all'.

addresses = []
nums = []
run = True

def rem_dupl(seq):
    seen = set()
    seen_add = seen.add
    return [x for x in seq if not (x in seen or seen_add(x))]

while run:
    x = str(input("Computer numbers: "))

    if x == 'done':
        run = False
    elif x == 'all':
        nums = [x for x in range(1, 70)]
        run = False
    elif int(x) > 0 and int(x) < 70:
        nums.append(x)
    else:
        print "Out of range"
else:
    nums_finished = rem_dupl(nums)

    for i in nums_finished:
        addresses.append("192.168.0." + str((int(i) + 10)))

    temp = open('hosts', 'w').close()

    file = open('hosts.txt', 'w')
    for item in addresses:
        print>>file, item

   file.close

Any thoughts, constructive criticism, apparent issues, or redundancies?

As suggested in the comments, I would strongly suggest looking at a proper configuration management tool such as Ansible or Puppet (both free) instead of rolling your own. That will probably save you a lot of time, once you’ve done the initial ramp-up.

Here are some comments on the code presented:

There aren’t any comments or documentation. It’s hard to tell if this script is behaving correctly, because it’s not written anywhere. (Although the description on the question is good, it would be better if it was embedded in the script itself.)
Use full names in variables; don’t skimp out with abbreviations. It makes the code much easier to read, and characters are cheap. For example: remove_duplicates instead of rem_dupl.

Speaking of that function, here’s a cleaner way to write it:

def remove_duplicates(sequence):
    return list(set(sequence))

In the while run: loop, rather than having the sentinel variable run, it would be nicer to use break statements to get out of the loop. Something like:

while True:
    x = ...
    if x == 'done':
        break
    elif x == 'all':
        ...
        break
    else:
        ...

This is one less variable to understand and track (for the human reading your code, that is!)
In Python 2, range() already returns a list, so

nums = [x for x in range(1, 70)]

can be reduced to

nums = range(1, 70)

I suspect you probably meant

nums = [str(x) for x in range(1, 70)]

because then nums is always a list of strings, however it gets populated.
Rather than creating addresses as an empty list and populating it one-by-one, you could use a list comprehension:

addresses = ["192.168.0." + str((int(i) + 10)) for i in set(nums)]

Note that in this case, we can just deduplicate with set() – we don’t even need to recast to a list to put it in our list comprehension.
It’s not clear what the temp variable is being used for, as we never seem to do anything with the hosts file.
Rather than using the f = open(path); ...; f.close() paradigm, it’s better to use the with statement like so:

with open('hosts.txt', 'w') as f:
    f.write('\n'.join(addresses))

This ensures that the file will always be closed correctly, even if something goes wrong while you’re trying to write to it.

(For example, your file.close is missing the parentheses to actually call the close() method, so this file will never be closed.)

Note also that it’s generally considered bad practice to use a builtin as a variable name. This can be a source of fun and weird bugs, so avoid it unless absolutely necessary.

And an ancillary thought:

As presented, the hosts will be written in the order supplied. Would it be better to sort the output, so that you can easily skim the list and spot missing entries? Not a must, just an idea.


Basic text editor in Python with Tkinter I have finished writing my nice and little text editor in Python with Tkinter.
I tried to achieve the best user experience that I could.

I want this code to be 2/3 compatible, I tested it in both versions but if you could test it out again in the 2 versions too it would very knd of you.
I did not use a class on purpose because I think that there is no need for it in such a small project (< 100 lines), do you think I should use classes also in small projects like this?
Is the user going to use this without problems?
Is the code going to be readable if I look back at it after, let's say, a year?
Do I use good names?
I did not use a __name__ guard because this file is not meant to be imported, do you think I should use it anyway?
What other features could I implement?

The code:

try:
    import tkinter as tk
except:
    import Tkinter as tk

try:
    import tkMessageBox as pop_up
except:
    import tkinter.tkMessageBox as pop_up

import time

EMPTY_TITLE_ERROR_MESSAGE_SAVE = "Please write the name of the file you want to save in the given field."
EMPTY_TITLE_ERROR_MESSAGE_OPEN = "Please write the name of the file you want to open in the given field."
FILE_NOT_FOUND_ERROR_MESSAGE = "No file with the given title was found, remember that this text editor can only read files in its directory."
SAVING_SUCCESS_MESSAGE = "Your text is now stored in the {filename} file"
SIGNATURE_TXT_NOT_FOUND_MESSAGE = "Please be sure that the file you want to open exists and that it is in the same folder of this editor."

def _open():
    if not file_title.get():
        pop_up.showerror("Title is empty.",EMPTY_TITLE_ERROR_MESSAGE_OPEN)
        return 1

    if not ".txt" in file_title.get():
        filename = file_title.get() + ".txt"

    try:
        with open(filename) as f:
            main_text.delete("1.0",tk.END)
            main_text.insert(tk.INSERT, f.read(), "a")
    except IOError:
        pop_up.showerror("File not found.",FILE_NOT_FOUND_ERROR_MESSAGE)

def save():
    if not file_title.get():
        pop_up.showerror("No title.",EMPTY_TITLE_ERROR_MESSAGE_SAVE)
        return 1

    if not ".txt" in file_title.get():
        filename = file_title.get() + ".txt"

    with open(filename,"w+") as f:
        f.write(main_text.get(1.0,tk.END))
    pop_up.showinfo("File saved succesfully.",SAVING_SUCCESS_MESSAGE.format(filename=filename))

def add_date():
    full_date = time.localtime()
    day = str(full_date.tm_mday)
    month = str(full_date.tm_mon)
    year = str(full_date.tm_year)
    date = "\n"+day+'/'+month+'/'+year
    main_text.insert(tk.INSERT, date, "a")

def add_signature():
    try:
        with open("signature.txt") as f:
            main_text.insert(tk.INSERT, "\n"+f.read(), "a")
    except IOError:
        MESSAGE = SIGNATURE_TXT_NOT_FOUND_MESSAGE
        pop_up.showerror("\"signature.txt\" not found.",MESSAGE)

root = tk.Tk()

menubar = tk.Menu(root)
menubar.add_command(label="Open", command=_open)
menubar.add_command(label="Save", command=save)
menubar.add_command(label="Add signature",command=add_signature)
menubar.add_command(label="Add date",command=add_date)

root.config(menu=menubar)

top = tk.Frame(root)
temp = tk.Label(root,text="Title:")
temp.pack(in_ = top,side=tk.LEFT)

file_title = tk.Entry(root)
file_title.pack(in_ = top,side=tk.RIGHT)

top.pack()

main_text = tk.Text(root)
main_text.pack()

tk.mainloop()

Style

Python has a guide style called PEP 8. If you have no good reason not to follow it, just follow it. You'll find various tools to check your code (pep8, pep8online.com/, etc) and even to fix it more or less automatically (autopep8).

Here's a list of problems detected :

txt_original.py:13:80: E501 line too long (105 > 79 characters)
txt_original.py:14:80: E501 line too long (105 > 79 characters)
txt_original.py:15:80: E501 line too long (141 > 79 characters)
txt_original.py:17:80: E501 line too long (138 > 79 characters)
txt_original.py:22:43: E231 missing whitespace after ','
txt_original.py:25:8: E713 test for membership should be 'not in'
txt_original.py:30:35: E231 missing whitespace after ','
txt_original.py:33:43: E231 missing whitespace after ','
txt_original.py:35:1: E302 expected 2 blank lines, found 1
txt_original.py:37:37: E231 missing whitespace after ','
txt_original.py:40:8: E713 test for membership should be 'not in'
txt_original.py:43:23: E231 missing whitespace after ','
txt_original.py:44:34: E231 missing whitespace after ','
txt_original.py:45:46: E231 missing whitespace after ','
txt_original.py:45:80: E501 line too long (95 > 79 characters)
txt_original.py:47:1: E302 expected 2 blank lines, found 1
txt_original.py:55:1: E302 expected 2 blank lines, found 1
txt_original.py:61:56: E231 missing whitespace after ','
txt_original.py:61:65: W291 trailing whitespace
txt_original.py:68:42: E231 missing whitespace after ','
txt_original.py:69:37: E231 missing whitespace after ','
txt_original.py:75:21: E231 missing whitespace after ','
txt_original.py:76:14: E251 unexpected spaces around keyword / parameter equals
txt_original.py:76:16: E251 unexpected spaces around keyword / parameter equals
txt_original.py:76:20: E231 missing whitespace after ','
txt_original.py:79:20: E251 unexpected spaces around keyword / parameter equals
txt_original.py:79:22: E251 unexpected spaces around keyword / parameter equals
txt_original.py:79:26: E231 missing whitespace after ','

And here's you code after most of the problems are fixed :

try:
    import tkinter as tk
except:
    import Tkinter as tk

try:
    import tkMessageBox as pop_up
except:
    import tkinter.tkMessageBox as pop_up

import time

EMPTY_TITLE_ERROR_MESSAGE_SAVE = "Please write the name of the file you want to save in the given field."
EMPTY_TITLE_ERROR_MESSAGE_OPEN = "Please write the name of the file you want to open in the given field."
FILE_NOT_FOUND_ERROR_MESSAGE = "No file with the given title was found, remember that this text editor can only read files in its directory."
SAVING_SUCCESS_MESSAGE = "Your text is now stored in the {filename} file"
SIGNATURE_TXT_NOT_FOUND_MESSAGE = "Please be sure that the file you want to open exists and that it is in the same folder of this editor."

def _open():
    if not file_title.get():
        pop_up.showerror("Title is empty.", EMPTY_TITLE_ERROR_MESSAGE_OPEN)
        return 1

    if ".txt" not in file_title.get():
        filename = file_title.get() + ".txt"

    try:
        with open(filename) as f:
            main_text.delete("1.0", tk.END)
            main_text.insert(tk.INSERT, f.read(), "a")
    except IOError:
        pop_up.showerror("File not found.", FILE_NOT_FOUND_ERROR_MESSAGE)

def save():
    if not file_title.get():
        pop_up.showerror("No title.", EMPTY_TITLE_ERROR_MESSAGE_SAVE)
        return 1

    if ".txt" not in file_title.get():
        filename = file_title.get() + ".txt"

    with open(filename, "w+") as f:
        f.write(main_text.get(1.0, tk.END))
    pop_up.showinfo(
        "File saved succesfully.",
        SAVING_SUCCESS_MESSAGE.format(
            filename=filename))

def add_date():
    full_date = time.localtime()
    day = str(full_date.tm_mday)
    month = str(full_date.tm_mon)
    year = str(full_date.tm_year)
    date = "\n" + day + '/' + month + '/' + year
    main_text.insert(tk.INSERT, date, "a")

def add_signature():
    try:
        with open("signature.txt") as f:
            main_text.insert(tk.INSERT, "\n" + f.read(), "a")
    except IOError:
        MESSAGE = SIGNATURE_TXT_NOT_FOUND_MESSAGE
        pop_up.showerror("\"signature.txt\" not found.", MESSAGE)

root = tk.Tk()

menubar = tk.Menu(root)
menubar.add_command(label="Open", command=_open)
menubar.add_command(label="Save", command=save)
menubar.add_command(label="Add signature", command=add_signature)
menubar.add_command(label="Add date", command=add_date)

root.config(menu=menubar)

top = tk.Frame(root)
temp = tk.Label(root, text="Title:")
temp.pack(in_=top, side=tk.LEFT)

file_title = tk.Entry(root)
file_title.pack(in_=top, side=tk.RIGHT)

top.pack()

main_text = tk.Text(root)
main_text.pack()

tk.mainloop()

As I am talking about tools, it is probably worth mentionning that you'll find various other tools to check your code.

pylint finds :

************* Module txt
C: 13, 0: Line too long (105/80) (line-too-long)
C: 14, 0: Line too long (105/80) (line-too-long)
C: 15, 0: Line too long (141/80) (line-too-long)
C: 17, 0: Line too long (138/80) (line-too-long)
C:  1, 0: Missing module docstring (missing-docstring)
W:  3, 0: No exception type(s) specified (bare-except)
F:  4, 4: Unable to import 'Tkinter' (import-error)
W:  8, 0: No exception type(s) specified (bare-except)
F:  7, 4: Unable to import 'tkMessageBox' (import-error)
E:  9, 4: No name 'tkMessageBox' in module 'tkinter' (no-name-in-module)
F:  9, 4: Unable to import 'tkinter.tkMessageBox' (import-error)
C: 20, 0: Missing function docstring (missing-docstring)
C: 29,31: Invalid variable name "f" (invalid-name)
C: 36, 0: Missing function docstring (missing-docstring)
C: 44,33: Invalid variable name "f" (invalid-name)
C: 52, 0: Missing function docstring (missing-docstring)
C: 61, 0: Missing function docstring (missing-docstring)
C: 63,38: Invalid variable name "f" (invalid-name)
C: 66, 8: Invalid variable name "MESSAGE" (invalid-name)
C: 69, 0: Invalid constant name "root" (invalid-name)
C: 71, 0: Invalid constant name "menubar" (invalid-name)
C: 80, 0: Invalid constant name "top" (invalid-name)
C: 81, 0: Invalid constant name "temp" (invalid-name)
C: 84, 0: Invalid constant name "file_title" (invalid-name)
C: 89, 0: Invalid constant name "main_text" (invalid-name)

but there is also pyflakes, pychecker, etc.

Logic around file name

The following logic :

if not ".txt" in file_title.get():
    filename = file_title.get() + ".txt"

seems wrong on many different levels :

It does not follow PEP8 and should be if ".txt" not in file_title.get()
You have the same hardcoded string in multiple places : you are pretty much asking to be biten at some point if you want to change this. What about TXT_EXTENSION = '.txt'.
You are calling file_title.get() for no good reason
You do not define filename if the condition in the if is evaluated to false. Maybe you meant :

TXT_EXTENSION = '.txt'
filename = file_title.get()
if TXT_EXTENSION not in filename:
    filename = filename + TXT_EXTENSION
You are changing "toto.tutu" to "toto.tutu.txt" but "toto.txt.tutu" does not change. If what you are interested in is extension, you should probably be using endswith.

You have the same logic in multiple places : you should probably move it in a function of its own :

def add_txt_extension_if_needed(filename):
    if not filename.endswith(TXT_EXTENSION):
        return filename + TXT_EXTENSION
    return filename

Date format

You can probably find in the documentation how to format a date as a string.

I have to go and I don't know tkinter but I guess other people will have additional relevant comments.


Hanoi towers time exeeded I am having problem with a simple Hanoi tower program.  I need to do something to reduce its time.

#include <stdio.h>
#include <fstream>

using namespace std;

int hanojus(int n, char a, char b, char c) {

    ofstream fr("hanoj.out",ios::app);
    if (n == 1)
        fr<<a<< " -> " <<c<<endl;
    else {
         hanojus(n-1, a, c, b);
         fr<<a<< " -> " <<c<<endl;
         hanojus(n-1, b, a, c);
        }
}
int main()
{
    ifstream fd("hanoj.in");
    int sk;
    fd>>sk;
    hanojus(sk, 'A', 'B', 'C');
}

Try moving the ofstream outside of the method so that you're not creating it every time you make a recursive call.

Like this:

#include <stdio.h>
#include <fstream>

using namespace std;

int hanojus(int n, char a, char b, char c, ofstream& fr)
{
    if (n == 1)
        fr<<a<< " -> " <<c<<endl;
    else
    {
        hanojus(n-1, a, c, b, fr);
        fr<<a<< " -> " <<c<<endl;
        hanojus(n-1, b, a, c, fr);
    }
}

int main()
{
    ifstream fd("hanoj.in");
    ofstream fr("hanoj.out",ios::app);
    int sk;
    fd>>sk;
    hanojus(sk, 'A', 'B', 'C', fr);
}


Add currency fields to get a subtotal, then add other values but only if the relevant checkbox is checked I have some currency fields which need to be added together. However, some of them have a checkbox nearby. If this is ticked, the field is added to the subtotal, if not, nothing is added.

I'm currently using the code below, which works, but doesn't seem very elegant or easy to maintain. I'm sure there is an easier way but I'm struggling to think of any other methods to use. There will be more checkboxes added later on, which will only lengthen the code. The below example is for two checkboxes.

decimal ditem1Cost = 0;
bool item1Checked = false;
if (Item1RateYN.Checked) {
    if (decimal.TryParse(item1Cost.Value.ToString(), out ditem1Cost))
        item1Checked = true;
}

decimal ditem2Cost = 0;
bool item2Checked = false;
if (Item2RateYN.Checked) {
    if (decimal.TryParse(item2Cost.Value.ToString(), out ditem2Cost))
        item2Checked = true;
}
//Add up some values which are always part of the subtotal
decimal subtotals = val1 + val2 + val3 + val4 + val5 + val6 + val7 + val8;

//Only add the ones which are near the checkboxes, if the checkbox is checked
subtotals = item1Checked ? subtotals + ditem1Cost : subtotals;
subtotals = item2Checked ? subtotals + ditem2Cost : subtotals;

//Add up some values which are always part of the subtotal
decimal subtotals = val1 + val2 + val3 + val4 + val5 + val6 + val7 + val8;

decimal ditem1Cost;
if (Item1RateYN.Checked) {
    decimal.TryParse(item1Cost.Value.ToString(), out ditem1Cost);
    subtotals+=ditem1Cost;
}
decimal ditem2Cost;
if (Item2RateYN.Checked) {
    decimal.TryParse(item2Cost.Value.ToString(), out ditem2Cost);
    subtotals+=ditem2Cost;
}

Is slightly better.

If using C#7 remove decimal ditem1Cost = 0; and replace decimal.TryParse(item2Cost.Value.ToString(), out ditem1Cost) with decimal.TryParse(item2Cost.Value.ToString(), out decimal cost)


Fantasy Game Inventory - Automate the Boring Stuff with Python Below is my code for the Fantasy Game Inventory problem from Chapter 5 of Automate the Boring Stuff with Python. Is there anything I can do to make it cleaner or more efficient?
The code starts out with a dictionary "inv" that contains the player's current inventory as well as a list "dragon_loot" that contains the items from the dead dragon. The goal of the code is to combine the list with the dictionary while increasing the quantity of duplicate items.
As a note, I am  using the starting list from the previous question simply because it seems impossible to kill a dragon without possessing a weapon :)

You are creating a fantasy video game. The data structure to model the player’s inventory will be a dictionary where the keys are string values describing the item in the inventory and the value is an integer value detailing how many of that item the player has. For example, the dictionary value {'rope': 1, 'torch': 6, 'gold coin': 42, 'dagger': 1, 'arrow': 12} means the player has 1 rope, 6 torches, 42 gold coins, and so on.
List to Dictionary Function for Fantasy Game Inventory
Imagine that a vanquished dragon’s loot is represented as a list of strings like this:
dragonLoot = ['gold coin', 'dagger', 'gold coin', 'gold coin', 'ruby']
Write a function named addToInventory(inventory, addedItems), where the inventory parameter is a dictionary representing the player’s inventory (like in the previous project) and the addedItems parameter is a list like dragonLoot. The addToInventory() function should return a dictionary that represents the updated inventory. Note that the addedItems list can contain multiples of the same item. Your code could look something like this:

def addToInventory(inventory, addedItems):
    # your code goes here

inv = {'gold coin': 42, 'rope': 1}
dragonLoot = ['gold coin', 'dagger', 'gold coin', 'gold coin', 'ruby']
inv = addToInventory(inv, dragonLoot)
displayInventory(inv)

The output of the following code is:

Inventory:
1 rope
6 torch
45 gold coin
2 dagger
12 arrow
1 ruby

Total number of items: 67

Thank you in advance. I have learned a lot from the comments I received for my previous questions!

from collections import Counter

# players current inventory before killing dragon
inv = {'rope': 1, 'torch': 6, 'gold coin': 42, 'dagger': 1, 'arrow': 12}

# items from the slain dragon
dragon_loot = ['gold coin', 'dagger', 'gold coin', 'gold coin', 'ruby']

# this function prints the players inventory in a list and reports the total items
def display_inventory(inventory):
    print("Inventory:")
    item_total = 0
    for k, v in inventory.items():
        print(str(v) + " " + str(k))
        item_total = item_total + int(v)
    print("\nTotal number of items: " + str(item_total))

# this function adds the dragon's stuff to the players inventory
def add_to_inventory(inventory, added_items):
        added_items_dict ={i:added_items.count(i) for i in added_items}
        inv = Counter(inventory) + Counter(added_items_dict)
        return inv

inv = add_to_inventory(inv, dragon_loot)
display_inventory(inv)

Style

I suggest you check PEP0008 https://www.python.org/dev/peps/pep-0008/ the official Python style guide for writing a more Pythonic code and Flake8 http://flake8.pycqa.org/en/latest/ for style enforcement. The following goes accordingly:

Blank lines: Surround top-level function and class definitions with two blank lines. Method definitions inside a class are surrounded by a single blank line.
Extra blank lines may be used (sparingly) to separate groups of related functions. Blank lines may be omitted between a bunch of related one-liners (e.g. a set of dummy implementations).

No blank lines between the function and what's above:

# this function prints the players inventory in a list and reports the total items
def display_inventory(inventory):

and same goes to the other function.
f-strings: Since you're using Python 3.x I can tell from the print statements F-strings provide a way to embed expressions inside string literals, using a minimal syntax. It should be noted that an f-string is really an expression evaluated at run time, not a constant value. You could use f-strings for readability enhancement in the following way:

This line:

print("\nTotal number of items: " + str(item_total))

is written:

print(f'\nTotal number of items: {item_total!s}')

as well as:

print(str(v) + " " + str(k))

is written:

print(f'{v!s} {k!s}')

Docstrings: Python documentation strings (or docstrings) provide a convenient way of associating documentation with Python modules, functions, classes, and methods. ... A docstring is simply a multi-line string, that is not assigned to anything. It is specified in source code that is used to document a specific segment of code. You should include docstrings to your functions indicating what they do and what they return.

Example:

def add_to_inventory(inventory: dict, added_items: list):
    """Update and return inventory."""

And note the type hints to indicate the types of these parameters.
Indentation: your function add_to_inventory body is over indented (8 spaces instead of 4) and 4 spaces is the common convention in Python.

Code

A function returns: functions are meant to return, not to print.

def display_inventory(inventory):
    print("Inventory:")
    item_total = 0
    for k, v in inventory.items():
        print(str(v) + " " + str(k))
        item_total = item_total + int(v)
    print("\nTotal number of items: " + str(item_total))

And since there is no particular sophisticated printing pattern required, the display function can be replaced by print() statement(s).
Augmented assignment: Python supports augmented assignment using +=

item_total = item_total + int(v) is written: item_total += int(v)
main guard: Use if __name__ == '__main__': at the end of your script and call your functions from there. to allow the module to be imported by other modules without running the whole script.

And the code can be a one-liner:

from collections import Counter

def update_inventory(inventory: dict, new_items: list):
    """Update and return inventory."""
    return Counter(inventory) + Counter(new_items)

if __name__ == '__main__':
    inventory = {'rope': 1, 'torch': 6, 'gold coin': 42, 'dagger': 1, 'arrow': 12}
    dragon_loot = ['gold coin', 'dagger', 'gold coin', 'gold coin', 'ruby']
    new_inventory = update_inventory(inventory, dragon_loot)
    for item, number in new_inventory.items():
        print(f'{number} {item}')
    print(f'\nTotal number of items: {sum(new_inventory.values())}')


Time limit exceeded for SPOJ problem "Prime Generator" Input

  The input begins with the number \$t\$ of test cases in a single line
  (\$t \le 10\$). In each of the next \$t\$ lines there are two numbers \$m\$ and \$n\$ (\$1 \le m \le n \le 1000000000\$, \$n-m \le 100000\$) separated by a space.

Output

  For every test case print all prime numbers \$p\$ such that \$m \le p \le n\$,
  one number per line, test cases separated by an empty line.

Example

Input:

2
1 10
3 5

Output:

2
3
5
7

3
5

I am trying my best to speed up this code, but I don't know any better way of doing it.

T = int(raw_input())
x= ' '
for test in xrange(T):
    minim,maxim = map(int,raw_input().split())
    if minim == 1:
        minim += 1
    for number in xrange(minim,maxim+1):
        if number == 2:
            print number
        for i in xrange(2,number):
            if number % i == 0:
                break
            elif i == number-1:
                print number
    print x

Disclaimer: I haven't tried the challenge myself so it will be pretty generic advices.

A bit of style

Python has a style guide called PEP 8. You'll find various tools to check that your code complies to it such as pep8, various tools to fix your code (such as autopep8). I also highly recommand testing other tools to check your code such as pylint, pychecker, pyflakes...

Problems in your code are mostly related to whitespace but you might want to get the habit of doing it right.

Also, as far as style is concerned : the x variable does not seem to be very useful (its name does not convey much meaning and the value does not change) ; the variable test is not used as such, the convention is to name throw-away values _.

A bit of scaffolding

A piece of code like the one you've submitted is quite hard to test as it is. Indeed, the only way to play with it is with input/output. This makes it quite hard to write automatic tests to ensure you do not break the algorithm but also to write performance tests to make your optimisations actually are optimisations.

Let's try to separate the concerns to have the actual algorithm on its own and the input/ouput in a different place.

Here's what I have done : the function takes a list of tuple (min, max) and returns a list of list so that each list corresponds to the list of primes. Then it is just a matter of pretty printing to solve the challenge.

I have converted the test in the example and some more to automatic tests :

def algo(input_val):
    sol = []
    for minim, maxim in input_val:
        primes = []
        if minim == 1:
            minim += 1
        for number in xrange(minim, maxim + 1):
            if number == 2:
                primes.append(number)
            for i in xrange(2, number):
                if number % i == 0:
                    break
                elif i == number - 1:
                    primes.append(number)
        sol.append(primes)
    return sol

def tests():
    assert algo([(1, 10), (3, 5)]) == [[2, 3, 5, 7], [3, 5]]
    assert algo([(5, 100)]) == [[5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41, 43, 47, 53, 59, 61, 67, 71, 73, 79, 83, 89, 97]]
    assert algo([(5, 100)] * 100) == [[5, 7, 11, 13, 17, 19, 23, 29, 31, 37, 41, 43, 47, 53, 59, 61, 67, 71, 73, 79, 83, 89, 97]] * 100
    algo([(10000, 50000)])

def main():
    input_val = [map(int, raw_input().split()) for _ in xrange(int(raw_input()))]
    output_val = algo(input_val)
    print(output_val)

if __name__ == '__main__':
    tests()

Better algorithms

At the moment, algo([(10000, 50000)]) is already very slow, there is no way you'll get to the extreme values in a correct time. You'll have to improve algorithms.

Better check for primeness

At the moment, when checking whether n is prime, you consider all potential divisors up to n. It is not worth the pain at least one divisor would be smaller or equal to the square root of n.

Also, it might be worth extracting the corresponding logic in a function on its own for clarity purposes (it took me a while to understand that minim += 1 was a premature optimisation).

Once it is done, you can realise that you can do even better than this by considering only odd numbers if the number is not divisible by 2 but I won't care about this yet.

The code is already much faster and looks like :

def is_prime(n):
    """Checks if a number is prime."""
    if n < 2:
        return False
    return all(n % i for i in range(2, int(math.sqrt(n)) + 1))

def algo(input_val):
    sol = []
    for minim, maxim in input_val:
        primes = []
        for number in xrange(minim, maxim + 1):
            if is_prime(number):
                primes.append(number)
        sol.append(primes)
    return sol

You can make it fancier by using list comprehension :

def algo(input_val):
    sol = []
    for minim, maxim in input_val:
        sol.append([n for n in xrange(minim, maxim + 1) if is_prime(n)])
    return sol

or even :

def algo(input_val):
    return [[n for n in xrange(minim, maxim + 1) if is_prime(n)]
            for minim, maxim in input_val
            ]

You can already handle much bigger values.

More tricks to handle bigger inputs

A trick to perform things quicker is to precompute values to be able to reuse them quickly. Such an algorithm for prime number is called the Sieve of Erathosthenes. You won't be able to compute it for the maximum values (n <= 1000000000) but it could be worth storing a list of primes up to sqrt(max) where max is the maximum max value in the input. If you do so, you'll perform the sieve for values up to maximum 31622 which is realistic. Then instead of checking all divisors, you can check only prime values (as primes gets rarer and rarer, I expect this to make the code faster).

Another trick might be to try to handle overlapping test cases by not performing the same actions many times.


Javascript Array processing If I have an array:

var names = ['John', 'Jim', 'Joe'];

and I want to create a new array from names which afterwards will look like:

newNames = ['John John', 'Jim Jim', 'Joe Joe']

What I came up with is the following:

var newNames = [];
var arr = null;
var loop = 0;

$.each(names, function (i, item) {
  arr = [];
  loop = 2;
  while (loop--) {
    arr.push(item);
  }
  newNames.push(arr.join(' '));
});

Seems like there should be a shorter, easier way to do this. As you can see we can repeat the names with a space n times. I'm experimenting with different ideas/concepts, having fun.

You could use the native Array.map method in modern browsers (see MDN)

const newNames = ['John', 'Jim', 'Joe'].map( name => `${name} ${name}`);
console.log(newNames);


Given hexadecimal rgb color, find the closest predefined color The following is a function that accepts a hex value (with or without a #, 3 or 6 character) and returns another hex value that is "closest" to it from a set of static hex values defined within the function. It involves a conversion to RGB.

How do I make this function more "elegant"? This is more of a question in general JS syntax and style, but I figured this was a good general example to present. I'd imagine I need to include some kind of nested functions, anonymous functions or closures that handle each part of the algorithm.

Here's the code, taking colors from Google's logo, for instance:

function nearestHex(hex) {

  var hexToRgb = function(hex) {
    var shortRegEx = /^#?([a-f\d])([a-f\d])([a-f\d])$/i;
    hex = hex.replace(shortRegEx, function(full, r, g, b) {
      return [r, r, g, g, b, b].join();
    });
    var longRegEx = /^#?([a-f\d]{2})([a-f\d]{2})([a-f\d]{2})?/i;
    var rgbArray  = longRegEx.exec(hex);
    var rgbObj = rgbArray ? {
      r: parseInt(rgbArray[1], 16),
      g: parseInt(rgbArray[2], 16),
      b: parseInt(rgbArray[3], 16)
    } : null;
    return rgbObj;
  }();

  var closestHexFromRgb = function(rgbObj) {
    if (!rgbObj) {
      throw new Error("The hex you provided is not formatted correctly. Please try in a format such as '#FFF' or '#DDFFDD'.");
    };

    var staticColors = [
      { color: 'red',    hex: '#EA4235', rgb: { r: 234, g: 66,  b: 53  } },
      { color: 'green',  hex: '#34A853', rgb: { r: 512  g: 168, b: 83  } },
      { color: 'blue',   hex: '#4285F4', rgb: { r: 66,  g: 133, b: 244 } },
      { color: 'yellow', hex: '#FBBC05', rgb: { r: 251, g: 188, b: 5   } }
    ];
    var minDistance = Number.MAX_SAFE_INTEGER;
    var nearestHex = null;

    for (var i=0; i<staticColors.length; i++) {
      var currentColor = staticColors[i];
      var distance = Math.sqrt(
        Math.pow((rgbObj.r - currentColor.rgb.r), 2) +
        Math.pow((rgbObj.g - currentColor.rgb.g), 2) +
        Math.pow((rgbObj.b - currentColor.rgb.b), 2)
      );
      if (distance < minDistance) {
        minDistance = distance;
        nearestHex = currentColor.hex;
      }
    };
    return nearestHex;
  }();

  closestHexFromRgb(hexToRgb(hex));

};

Fixing and improving the short regex handling

This won't work as intended:

hex = hex.replace(shortRegEx, function(full, r, g, b) {
  return [r, r, g, g, b, b].join();
});

The default separator of join is a comma, so this will join the hexadecimal digits by commas, when you want empty string: .join("")

In any case, instead of the anonymous function + array + joining,
it will be simpler to use the capture groups of the regex in the replacement string:

hex = hex.replace(shortRegEx, "$1$1$2$2$3$3");

Don't repeat yourself

Instead of repeating parseInt(x, 16) multiple times here:

  r: parseInt(rgbArray[1], 16),
  g: parseInt(rgbArray[2], 16),
  b: parseInt(rgbArray[3], 16)

It would be better to create a function for it.

The same goes for this snippet in the distance calculation:

    Math.pow((rgbObj.r - currentColor.rgb.r), 2) +
    Math.pow((rgbObj.g - currentColor.rgb.g), 2) +
    Math.pow((rgbObj.b - currentColor.rgb.b), 2)

When defining the static colors here,
it's not obvious that the rgb values are correct:

  { color: 'red',    hex: '#EA4235', rgb: { r: 234, g: 66,  b: 53  } },
  { color: 'green',  hex: '#34A853', rgb: { r: 512  g: 168, b: 83  } },
  { color: 'blue',   hex: '#4285F4', rgb: { r: 66,  g: 133, b: 244 } },
  { color: 'yellow', hex: '#FBBC05', rgb: { r: 251, g: 188, b: 5   } }

You could make it obvious by reusing the hexToRgb function to create the rgb values.


Inserting JSON array data into a MySQL database using PHP Here I am parsing a JSON array and inserting it into a MySQL database. The JSON Array comes from my android code.

This is how my JSON array looks like:

["{custInfo=Ujwal  9975022560, rate=24000, weight=21.00000, desc=GENTS ANGTHI 22k NO STONE, makingAmt=200, vat=RS.3064.38, itemTotal=51073, sum_total=RS.156283.38, barcode=BQSP78BB, net_rate=24200, date=2015-11-30, invoiceNo=1, bill_type=Invoice}",
"{custInfo=Ujwal  9975022560, rate=24000, weight=21.00000, desc=GENTS ANGTHI 22k NO STONE, makingAmt=200, vat=RS.3064.38, itemTotal=51073, sum_total=RS.156283.38, barcode=BQSP78BB, net_rate=24200, date=2015-11-30, invoiceNo=1, bill_type=Invoice}",
"{custInfo=Ujwal  9975022560, rate=24000, weight=21.00000, desc=GENTS ANGTHI 22k NO STONE, makingAmt=200, vat=RS.3064.38, itemTotal=51073, sum_total=RS.156283.38, barcode=BQSP78BB, net_rate=24200, date=2015-11-30, invoiceNo=1, bill_type=Invoice}"]

This is my php code to parse and insert the data.

<?php
require "init.php";
$json = file_get_contents('php://input');
//$data = json_decode($json,true);
// remove the ,true so the data is not all converted to arrays
$data = json_decode($json);
// Now process the array of objects
foreach ( $data as $inv ) {
    $custInfo = $inv->custInfo;
    $rate =     $inv->rate;
    $weight=    $inv->weight;
    $desc=      $inv->desc;
    $makingAmt= $inv->makingAmt;
    $vat=       $inv->vat;
    $itemTotal= $inv->itemTotal;
    $sum_total= $inv->sum_total;
    $barcode=   $inv->barcode;
    $net_rate=  $inv->net_rate;
    $date=      $inv->date;
    $invoiceNo= $inv->invoiceNo;
    $bill_type= $inv->bill_type;
    $sql = "INSERT INTO selected_items
             (custInfo, invoiceNo, barcode, desc,
              weight, rate, makingAmt,net_rate,
              itemTotal,vat,sum_total,bill_type,date)
            VALUES
             ('$custInfo','$invoiceNo','$barcode','$desc',
              '$weight','$rate','$makingAmt','$net_rate',
              '$itemTotal','$vat','$sum_total','$bill_type','$date')";
    $res = mysqli_query($sql,$con);
echo $res;
    if(!$res){
        $result = new stdClass();
        $result->status = false;
        $result->msg = mysql_error();
        echo json_encode($result);
        exit;
    }
}
?>

You are open to SQL Injection. To prevent this, use prepared statements.

You also should not mix mysql_ and mysqli_ functions; always use mysqli_ (or PDO).

Having variables that are only accessed once is only useful in a limited number of cases, eg when you want to give something a nice name to increase readability. But this is not the case with your variables, so they are really not needed and actually decrease readability. Jut use $inv->custInfo etc directly. This will also severely shorten your code.


POST variable analysis contains JSON format string PHP I am verifying certain codes that work well and I don't know what else to optimize in them or what other tests to do: in this case it is a php script that takes the data of the $_POST variable analyzes them and converts them into an array, this is since in some cases the post field may contain a JSon string.

I leave a functional example where I have manually set the $_POST variable to do the Demonstration:

https://wtools.io/php-sandbox/bqsT

Original Code example:

<?php

#Example Array

$_POST = array(
    "valor1" => 1200,
    "valor2" => "texto",
    "valor3" => true,
    "valor4" => '{"seclvl_text":"datp","seclvl_boolean":"false"}',
);
#Validate Function for Json
function ValidateJson($Var) {
    if (!is_array($Var)) {
        return ((json_decode($Var) != null) AND (is_object(json_decode($Var)) OR is_array(json_decode($Var)))) ? true : false;
    } else {
        return false;
    }
}
#Parse Function
function buildVirtualData($data) {
    if (is_array($data)) {
        $result = [];
        foreach ($data as $key1 => $val1) {
            $valJson = ValidateJson($val1);
            if ($valJson) {
                $jsonObj       = json_decode($val1, true);
                $result[$key1] = buildVirtualData($jsonObj);
            } elseif ($valJson == false && is_array($val1)) {
                foreach ($val1 as $key2 => $val2) {
                    $result[$key1][$key2] = buildVirtualData($val2);
                }
            } else {
                if ($val1 === 'true') {
                    $val1 = true;
                } else if ($val1 === 'false') {
                    $val1 = false;
                }
                $result[$key1] = $val1;
            }
        }
        return $result;
    } else {
        if (ValidateJson($data)) {
            $jsonObj = json_decode($data, true);
            return buildVirtualData($jsonObj);
        } else {
            return $data;
        }
    }
}
# call to Function:
$data = buildVirtualData($_POST);
echo '<pre>';
echo var_dump($data);
echo '</pre>';

this parse have the feature that convert any text true or false to Boolean.

I think you're are definitely on the right track here. I spent a good deal of time trying to refactor, here's what I came up with

#Validate Function for Json
function jsonDecodeAndValidate($var, $assoc = false) {
    if(!is_string($var))
        return false;
    if(!$decoded = json_decode($var, $assoc))
        return false;
    if(!is_object($decoded) && !is_array($decoded))
        return false;
    return $decoded;
}
#Parse Function
function buildVirtualData($data) {
    if (is_array($data) || is_object($data)) {
        foreach ($data as $key1 => $val1) {

            if(is_array($val1)){
                foreach ($val1 as $key2 => $val2) {
                    $result[$key1][$key2] = buildVirtualData($val2);
                }
            }
            else if($decoded = jsonDecodeAndValidate($val1)){
                $result[$key1] = buildVirtualData($decoded);
            } else {
                if(in_array($val1, ['true', 'false']))
                    $val1 = filter_var($val1, FILTER_VALIDATE_BOOLEAN);
                $result[$key1] = $val1;
            }
        }
        return $result;
    } else {
        if ($decoded = jsonDecodeAndValidate($data, true)) {
            return buildVirtualData($decoded);
        } else {
            return $data;
        }
    }
}

At the very least this will save you some calls to json_decode as the decoding is done once per iteration.

EDIT: I took another stab at it. I think you can boil it down to this.

#decodeand validate
function jsonDecodeAndValidate($var) {
    if(!is_string($var))
        return $var;
    if(!$decoded = json_decode($var, true))
        return $var;
    if(!is_array($decoded))
        return $var;
    return $decoded;
}
#Parse Function
function buildVirtualData($data, &$build) {
    $data = jsonDecodeAndValidate($data);
    if(is_array($data) || is_object($data)){
        foreach($data as $key => $value){
            buildVirtualData($value, $build[$key]);
        }
    } else {
        if ($data === 'true')
          $data = true;
        if ($data === 'false')
          $data = false;
        $build = $data;
    }
}
# call to Function:
$build = [];
buildVirtualData($_POST, $build);
echo '<pre>';
echo json_encode($build, JSON_PRETTY_PRINT);
echo '</pre>';

You just have to declare your array outside of the function.


Beautiful Binary String Online challenge on Hacker Rank.

  Alice has a binary string, B, of length n. She thinks a binary
  string is beautiful if and only if it doesn't contain the substring
  010.

  In one step, Alice can change a 0 to 1 a  (or vice-versa). Count
  and print the minimum number of steps needed to make Alice see the
  string as beautiful.

  Input Format

  The first line contains an integer, n (the length of binary string
  B).  The second line contains a single binary string, B, of length
  n.

  Output Format

  Print the minimum number of steps needed to make the string beautiful.

public class Solution {
    private static int countChange(String text, String pattern) {
        int textLen = text.length();
        int patternLen = pattern.length();

        char[] chars = text.toCharArray();
        int count = 0;
        for (int i = 0; i <= textLen - patternLen; i++) {
          int j = 0;
          while (j < patternLen && chars[i+j] == pattern.charAt(j)) {
            j++;
          }
          if (j == patternLen) {
            count++;
            chars[i+2] = '1';
          }
        }
        return count;
    }

    public static void main(String[] args) {
        Scanner in = new Scanner(System.in);
        int n = in.nextInt();
        String B = in.next();
        System.out.println(countChange(B, "010"));
    }
}

Generic pattern argument but hardcoded assumptions
It's nice that you added a generic pattern argument, but your code depends on the pattern being "010" here:

      if (j == patternLen) {
        count++;
        chars[i+2] = '1';
      }

Here, you have hardcoded both a 2 and a '1'.  Your program would crash if the pattern were shorter than 3 characters and if i were the last index of the array.  Also, if the pattern were "101", setting the last character to '1' would not accomplish anything.
It would be better to just skip the rest of the pattern:
      if (j == patternLen) {
        count++;
        i += patternLen - 1;
      }


Releasing retained objects inside dealloc When releasing objects that might have been retained during the app's lifetime, how do you check if the object really exists and prevent releasing a nil object?

Here's how I'm doing it:

- (void)dealloc{
    if(account_)
        [account_ release];
    account_ = nil;
    [super dealloc];
}

- (void)dealloc{
    if(account_)

You don't need to do this. Objective-C ignores attempts to call methods on nil objects. So you call release and nothing will happen if its already nil

        [account_ release];
    account_ = nil;

Your object is being destroyed, there isn't a whole lot of point in setting values to nil

    [super dealloc];
}


SQL Server 2012: How do I eliminate multiple statements in this code to a more efficient, single statement? This code completes under a second, however its seems like I've taken the "Mr. Bean" way around getting there.

The goal is to get the average number of orders processed in a single day based on how many days exist in the current database. The loadcount column represents how many orders are processed in a single batch.  The batch is driven by the date and time during the day.

        -- // ditch the time to get the start/end date ranges uniform by date //--
    ;with makeuniform as
    (SELECT
           cast(startrangedatetime as date) as startdate
          ,cast(endrangedatetime as date) as enddate
          ,[LoadCount]
      FROM [audit].[Tracker]
    )
    -- // next, rollup all the loadcounts by uniform date // --
    select startdate,
           sum(loadcount) TotalOrders
      into #sumthedays
     from makeuniform
     group by startdate;

    -- // since the counts have been rolled up by day, get the table count // --
    declare @howmanydays int
    select  @howmanydays = count(*) from #sumthedays;

    -- // next get the average traffic per day // --

    select sum(TotalOrders) / @howmanydays
     from #sumthedays

     drop table #sumthedays

This could be simplified. You don't have to create a temporary table to get your unique day count. You can use the COUNT function on the StartDate column directly.

Try this:

SELECT
    TotalLoadCount / NULLIF(TotalDayCount, 0) AvgLoadCountByDay
FROM
(
SELECT
    COUNT(DISTINCT  CAST(StartDate as date)) TotalDayCount
    ,SUM(LoadCount) TotalLoadCount
FROM
    dbo.Tracker
) x

Obviously, you can write it directly like this as well:

SELECT
    SUM(LoadCount) / NULLIF(COUNT(DISTINCT  CAST(StartDate AS date)), 0) AvgLoadCountByDay
FROM
    dbo.Tracker

If my understanding of your problem is correct, this query should give you the same result. Let me know if it doesn't.

I personally prefer the first version with derived table as it helps with the readability of the query. You are not paying any additional cost because SQL Server will generate the same execution plan for both the queries.


DownloadFile Function (using LibCurl) Please tell me if this function is correct (I believe it to be), efficient, and clean.

Function to check:

::CURLcode CURL_DownloadFile( __in ::LPCSTR URL, __in ::LPCSTR Destination )
{
    ::CURL* Curl;
    ::CURLcode Return_CurlCode;
    std::FILE* File;

    if( ( Curl = ::curl_easy_init( ) ) )
    {
        File = std::fopen( Destination, "wb" );

        ::curl_easy_setopt( Curl, CURLOPT_URL, URL );
        ::curl_easy_setopt( Curl, CURLOPT_WRITEFUNCTION, WriteData );
        ::curl_easy_setopt( Curl, CURLOPT_WRITEDATA, File );

        Return_CurlCode = ::curl_easy_perform( Curl );
        std::fclose( File );
    } else { ( Return_CurlCode = CURLE_FAILED_INIT ); }

    return( Return_CurlCode );
};

Example of usage:

#include "Global.h"

::CURLcode CURL_DownloadFile( __in ::LPCSTR URL, __in ::LPCSTR Destination );

int main( void )
{
    if( CURL_DownloadFile( "http://stackoverflow.com/", "c:\\users\\xorr\\desktop\\stackoverflow.txt" ) != CURLE_OK )
        std::printf( "An error has occured...\n" );

    std::cin.get( );
    return( EXIT_SUCCESS );
    // Xorr@Hotmail.com
};

size_t WriteData( void *ptr, size_t size, size_t nmemb, std::FILE *stream ) {
    size_t written;
    written = fwrite(ptr, size, nmemb, stream);
    return written;
}

::CURLcode CURL_DownloadFile( __in ::LPCSTR URL, __in ::LPCSTR Destination )
{
    ::CURL* Curl;
    ::CURLcode Return_CurlCode;
    std::FILE* File;

    if( ( Curl = ::curl_easy_init( ) ) )
    {
        File = std::fopen( Destination, "wb" );

        ::curl_easy_setopt( Curl, CURLOPT_URL, URL );
        ::curl_easy_setopt( Curl, CURLOPT_WRITEFUNCTION, WriteData );
        ::curl_easy_setopt( Curl, CURLOPT_WRITEDATA, File );

        Return_CurlCode = ::curl_easy_perform( Curl );
        std::fclose( File );
    } else { ( Return_CurlCode = CURLE_FAILED_INIT ); }

    return( Return_CurlCode );
};

Your write function is incorrect:

size_t WriteData( void *ptr, size_t size, size_t nmemb, std::FILE *stream ) {
    size_t written;
    written = fwrite(ptr, size, nmemb, stream);
    return written;
}

CURL will abort the transfer if you do not return size*nmemb. So you need to take care to make sure that you actually write all the data provided.

Also this is the action of the default CURLOPT_WRITEFUNCTION so there is no need to write your own version.

You don't check if the File pointer is NULL

    File = std::fopen( Destination, "wb" );

This is a C function (even if you have used it from C++). This means you must check the return value to make sure the faction succeeded.

Don't artifically stop the application. You may forget to remove this and then it will make the program look like it is broken.

std::cin.get( );

If you run from the command line it is not needed. If you are running inside an IDE set up your IDE so that it does not close the output window when the application finishes.

If your program always succeeded. Then I prefer not to return an exit value from main(). The compiler in this one special case will plant a return 0; for you to indicate success. This is a good indication that you have designed your application so that it never fails.

return( EXIT_SUCCESS );

I only return a value if there is a potential for failure and I want to be able to indicate the possible failure states.


RFC on "Factory" code This is the first code of this type that I've attempted, so I'm wondering if anybody would be willing to critique it. It's "sort of" (I think) a factory class. I want to keep it as simple as possible (and still work, and be at least as elegant as Phyllis Diller - it doesn't have to be Elizabeth Taylor).

The situation is that we may eventually support any number of printer types (printing labels from a Windows CE device), but want to keep the client code to something as simple as:

var beltPrinter = new BeltPrinter();
beltPrinter.PrintLabel(price, description, barcode);

Each print job will consist of 0..N preparatory commands sent to the printer ("hey, wake up!") followed by 1..N print commands, followed by 0..N shutdown commands.

Based on that, does the following make sense? If not, what would you advise that I change?

interface IBeltPrinter
    {
        private List<string> printerSetupCmds;
        private List<string> printLabelCmds;
        private List<string> printerTeardownCmds;

        void PrintLabel();
    }

class BeltPrinter : IBeltPrinter
{
    if (<ZebraQL220>) // don't know yet how I will determine which printer is in use - perhaps from reading a config file
    {
        return ZebraQL220Printer;
    }
    else if (<ONeal>)
    {
        return ONealPrinter;
    }
}

class ZebraQL220Printer : IBeltPrinter
    {
        public void PrintLabel(string Price, string Desc, string Barcode)
        {
            var _price = Price;
            var _desc = Desc;
            var _barcode = Barcode;

            printerSetupCmds = new List<string>();
            printerSetupCmds.Add(''); // the format and commands, etc. will differ from other printers
            . . .
            printLabelCmds = new List<string>();
            printLabelCmds.Add('');
            . . .
            printerTeardownCmds = new List<string>();
            printerTeardownCmds.Add('');
            . . .

            foreach (string line in printerSetupCmds) {
    //send to ZebraQL220
            }
            foreach (string line in printLabelCmds) {
    //send to ZebraQL220
            }
            foreach (string line in printerTeardownCmds) {
    //send to ZebraQL220
            }
        }
    }

    class ONealPrinter : IBeltPrinter
    {
        public void PrintLabel(string Price, string Desc, string Barcode)
        {
            var _price = Price;
            var _desc = Desc;
            var _barcode = Barcode;

            printerSetupCmds = new List<string>();
            printerSetupCmds.Add(''); // the format and commands, etc. will differ from other printers
            . . .
            printLabelCmds = new List<string>();
            printLabelCmds.Add('');
            . . .
            printerTeardownCmds = new List<string>();
            printerTeardownCmds.Add('');
            . . .

            foreach (string line in printerSetupCmds) {
    //send to ONeal
            }
            foreach (string line in printLabelCmds) {
    //send to ONeal
            }
            foreach (string line in printerTeardownCmds) {
    //send to ONeal
            }
        }
    }

First off, I have to say this is a very clean and efficient design.

I'm hoping class IBeltPrinter BeltPrinter() is a typo, because I've never seen C# code like that.

Variable names for local variables (and parameters) should start with a lower case letter.

I would move the setup, labelCommands and tear down into their own methods.  It will keep the PrintLabel method much cleaner

public void PrintLabel(string price, string desc, string barcode)
{
    ExecuteSetup(/*variables*/);
    ExecuteCommands(/*variables*/);
    ExecuteTeardown(/*variables*/);
}

Come to think of it, you could have a base class that has the PrintLabel method, and virtualExecuteSetup,ExecuteCommands, andExecuteTeardown` methods, that have generic printer commands.  You could then extend that base class as required.

I would also consider putting the Factory definitions in an IDictionary.  This will eliminate having a potentially huge switch statement.

private readonly static IDictionary<[whatever], IBeltPrinter> SupportedPrinters = new Dictionary<[whatever], IBeltPrinter>
    {
        {[ZebraQL220], new ZebraQL220Printer()},
        {[ONeal], new ONealPrinter()},
    }

Then your factory call would be

public IBeltPrinter CreatePrinter([whatever] printer)
{
    if (!SupportedPrinters.ContainsKey(printer))
        throw new PrinterNotSupportedException(printer)

    return SupportedPrinters[printer];
}


2D matrix - splitting the matrix in two parts with equal number of elements The elements in the matrix are either 'M', 'G', 'P' or '.' and the algorith must write out all possible ways of splitting the matrix so it has equal number of M, G and P on each side. It's a task we've been given to do at home. We also had to use use the matrix as a parameter in the function and use it as a pointer in it. What would you change to make it more optimized?

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

int field_split(char *k, int r, int s, int *ptrP, int *ptrG, int *ptrM){
    int i, j, numberP = 0, numberG = 0, numberM = 0;
    int splitNumber = 0;
    int tmpP = 0, tmpG = 0, tmpM = 0;

    for (i = 0; i < r*s; i ++){
        switch(k[i]){
            case 'P':
                numberP ++;
                break;
            case 'M':
                numberM ++;
                break;
            case 'G':
                numberG ++;
                break;
        }
    }

    for (i = 0; i < r; i++){
        for (j = 0; j < s; j ++){
            switch(k[i * s + j]){
                case 'P':
                    tmpP ++;
                    break;
                case 'M':
                    tmpM ++;
                    break;
                case 'G':
                    tmpG ++;
                    break;
            }
        }
        if(tmpP == numberP / 2 && tmpM == numberM / 2 && tmpG == numberG / 2)
            splitNumber ++;
    }

    tmpP = tmpM = tmpG = 0;

    for (j = 0; j < s; j++){
        for (i = 0; i < r; i ++){
            switch(k[i * s + j]){
                case 'P':
                    tmpP ++;
                    break;
                case 'M':
                    tmpM ++;
                    break;
                case 'G':
                    tmpG ++;
                    break;
            }
        }
        if(tmpP == numberP / 2 && tmpM == numberM / 2 && tmpG == numberG / 2)
            splitNumber ++;
    }

    *ptrP = numberP;
    *ptrG = numberG;
    *ptrM = numberM;

    return splitNumber;
}

int main()
{
    int r, s, i, j;
    int splitNumber;
    int numberP, numberM, numberG;
    int tmpP = 0, tmpM = 0, tmpG = 0;
    int count = 1;

    scanf("%d %d", &r, &s);
    char field[r][s];

    for (i = 0; i < r; i ++){
        for(j = 0; j < s; j ++)
            scanf(" %c", &field[i][j]);
    }

    splitNumber = field_split(&field[0][0], r, s, &numberP, &numberG, &numberM);

    if (splitNumber > 0 ){

        printf("Ways of splitting the field: %d", splitNumber);

        for (i = 0; i < r; i ++){
            for(j = 0; j < s; j ++){
                switch(field[i][j]){
                    case 'P':
                        tmpP ++;
                        break;
                    case 'G':
                        tmpG ++;
                        break;
                    case 'M':
                        tmpM ++;
                        break;
                }
            }
            if(tmpP == numberP / 2 && tmpG == numberG / 2 && tmpM == numberM / 2){
                printf("\n%d. Horizontally, between row  %d and %d", count, i + 1, i + 2);
                count ++;
            }
        }

        tmpP = tmpG = tmpM = 0;

        for (j = 0; j < s; j ++){
            for(i = 0; i < r; i ++){
                switch(field[i][j]){
                    case 'P':
                        tmpP ++;
                        break;
                    case 'G':
                        tmpG ++;
                        break;
                    case 'M':
                        tmpM ++;
                        break;
                }
            }
            if(tmpP == numberP / 2 && tmpG == numberG / 2 && tmpM == numberM / 2){
                printf("\n%d. Vertically, between column %d and %d", count, j + 1, j + 2);
                count++;
            }
        }
    }

    else
        printf("Can't be split 50-50.");

    return 0;
}

Bug
If there are an odd number of M, P, or G, it should be impossible to split the matrix in half.  However, your program may erroneously think it is possible because of this type of comparison:

tmpP == numberP / 2

which can be true for example if tmpP == 2 and numberP == 5, due to the division rounding down.  Here is an example input that demonstrates this:
3 3
M M M
M M S
S P S

An easy way to fix this would be to compare like this instead:
tmpP * 2 == numberP

which doesn't suffer from any roundoff errors.  Or you could immediately return 0 possibilities if any of the three counts is odd.
Repetitive code
Your code in field_split() seems reasonable, and there isn't much that can be improved there.  However, you created a nearly exact duplicate of your code in main().  Perhaps you did this because you needed to print the total count of the answers before printing each individual answer.
A better alternative would be to generate the answers in field_split() and return them to main().  Then you could print the count of answers followed by each individual answer.  You not only reduce your code by half, you also don't have to traverse your matrix twice.
Sample rewrite
Here is how I would rewrite your code.  Some things I did:

Use helper functions to handle repetitive tasks.
Return all the ways the matrix can be split directly from field_split() to avoid having to traverse the matrix twice.
Short circuit the function if any of the counts are odd.
Cleaned up the printing code to add the proper newlines.
Added const keyword to k argument which is not modified by field_split().
Cleaned up spacing which was inconsistent in a few places.

The rewrite:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdbool.h>

#define MAX_SYMBOLS        3

// Counts GMP and adds to symCount[]
static void countSymbol(char symbol, int *symCount)
{
    switch (symbol) {
        case 'G':
            symCount[0]++;
            break;
        case 'M':
            symCount[1]++;
            break;
        case 'P':
            symCount[2]++;
            break;
    }
}

// Returns true if each count in curCount is half the count in totCount.
static bool isHalf(int *curCount, int *totCount)
{
    int i;

    for (i = 0; i < MAX_SYMBOLS; i++) {
        if (curCount[i] * 2 != totCount[i])
            return false;
    }
    return true;
}

// Returns the number of ways the matrix can be split horizontally or
// vertically, with each side having half of each symbol.
//
// WaysHorizontal must be an array of at least (columns-1) size.
// WaysVertical   must be array of at least (rows-1) size.
static int field_split(const char *k, int rows, int columns,
        int *pNumHorizontal, int *waysHorizontal, int *pNumVertical,
        int *waysVertical)
{
    int i                     = 0;
    int j                     = 0;
    int totSize               = rows * columns;
    int totCount[MAX_SYMBOLS] = {0};
    int curCount[MAX_SYMBOLS] = {0};
    int numHorizontal         = 0;
    int numVertical           = 0;

    // First count all the symbols.
    for (i = 0; i < totSize; i++) {
        countSymbol(k[i], totCount);
    }

    // If any count is odd, there are no answers.
    for (i = 0; i < MAX_SYMBOLS; i++) {
        if (totCount[i] & 1) {
            *pNumHorizontal = 0;
            *pNumVertical   = 0;
            return 0;
        }
    }

    // Find horizontal splits.
    for (i = 0; i < rows - 1; i++) {
        for (j = 0; j < columns; j++) {
            countSymbol(k[i*columns + j], curCount);
        }
        if (isHalf(curCount, totCount)) {
            waysHorizontal[numHorizontal++] = i;
        }
    }

    // Find vertical splits.
    memset(curCount, 0, sizeof(curCount));
    for (j = 0; j < columns - 1; j++) {
        for (i = 0; i < rows; i++) {
            countSymbol(k[i*columns + j], curCount);
        }
        if (isHalf(curCount, totCount)) {
            waysVertical[numVertical++] = j;
        }
    }

    *pNumHorizontal = numHorizontal;
    *pNumVertical   = numVertical;
    return numHorizontal + numVertical;
}

int main(void)
{
    int rows, columns, i, j;
    int splitNumber;

    scanf("%d %d", &rows, &columns);

    char field[rows][columns];
    int waysHorizontal[columns];
    int waysVertical[rows];
    int numHorizontal;
    int numVertical;
    int totalSplits;

    for (i = 0; i < rows; i++) {
        for (j = 0; j < columns; j++) {
            scanf(" %c", &field[i][j]);
        }
    }

    totalSplits = field_split(&field[0][0], rows, columns, &numHorizontal,
                    waysHorizontal, &numVertical, waysVertical);

    if (totalSplits > 0) {
        int splitIndex = 1;

        printf("Ways of splitting the field: %d\n", totalSplits);
        for (i = 0; i < numHorizontal; i++) {
            printf("%d. Horizontally, between row %d and %d\n", splitIndex++,
                    waysHorizontal[i]+1, waysHorizontal[i]+2);
        }
        for (i = 0; i < numVertical; i++) {
            printf("%d. Vertically, between column %d and %d\n", splitIndex++,
                    waysVertical[i]+1, waysVertical[i]+2);
        }
    } else {
        printf("Can't be split 50-50.\n");
    }

    return 0;
}


Switch case implementation in Java for an integer pair combination I have following Python code:

def get_subject_from_stream_id_and_subject_id(stream_id, subject_id):
    #(stream_id, subject_id): ("subject_name")
    return {
        (1, 1): "Accounts",
        (1, 2): "English",
        (1, 3): "Organization of Commerce",
        (2, 1): "Physics",
        (2, 2): "English",
        (2, 3): "Biology"
    }.get((stream_id, subject_id), "None")

In this code, I want to get subject name from the integer pair combination i.e. stream_id, subject_id e.g. (1, 2) is for English. It was implemented using a Python tuple.

I want to implement the same piece of code in Java.

Could someone write this in a better way in Java?

public String getSubjectFromStreamIdAndSubjectId(int streamId, int subjectId) {
  switch (streamId) {
    case 1:
        switch (subjectId) {
        case 1:
            return "Accounts";
        case 2:
            return "English";
        case 3:
            return "Organization of Commerce";
        default:
            return null;
        }

    case 2:
        switch (subjectId) {
        case 1:
            return "Physics";
        case 2:
            return "English";
        case 3:
            return "Biology";
        default:
            return null;
        }
    default:
        return null;
   }
}

Neither is good, but surely we can all agree that the Java example is way uglier. The biggest problem, however, is that both examples hard code data into code.

Separate data, i.e. stream and subject IDs and their titles into a data class and in that class implement code that accesses the data structure without detailed knowledge about the actual data. Responsibility of setting up the data structure to resemble your stream and subject numbering is left to a separate component (load it from file or set up in static code).

Whether the data class is a recursive structure or just a wrapper for a HashMap depends on the complexity of your data.


Game where two players take turns picking numbers until a target sum Working on the problem of sum game. Suppose we have consecutive numbers from 1 to n, and an expected sum number, there are two players, and each player tries to select one number in turn (once a number is selected by one player, the other player cannot select it again), and the selected numbers are summed together. The player who first get a sum which is >= expected sum wins the game. The question is trying to find if first player has a force win solution (means no matter what numbers 2nd player will choose each time, the first player will always win).

For example, if we have numbers from 1 (inclusive) to 5 (inclusive) and expected sum is 7, if the first player select 1, no matter what the 2nd player select, in the final the first player will always win.

Here is my code and my idea is, each player tries to see if select the largest number can win -- if not then selecting the smallest available (means not having been selected) number -- which gives the current player max chance to win in the following plays, and give the opponent player minimal chance to win in the following plays.

I think the above strategy is optimum for player 1 and player 2, if in this strategy, player 2 cannot win, it means player 1 could force win.

I want to review the code, and also if my thought of the algorithm (strategy) to resolve the problem is correct?

def try_best_win(numbers, flags, current_sum, expected_sum, is_first_player):
    if current_sum + numbers[-1] >= expected_sum:
        return is_first_player == True
    else:
        for i in range(len(flags)):
            # find the next smallest number
            if flags[i] == False:
                flags[i] = True
                return try_best_win(numbers, flags, current_sum+numbers[i], expected_sum, not is_first_player)

if __name__ == "__main__":
    print try_best_win([1,2,3,4,5], [False]*5, 0, 6, True) #False
    print try_best_win([1,2,3,4,5], [False]*5, 0, 7, True) #True

Prelude

Firstly, you might want to ask on Math Stack Exchange if there is any analysis. This game looks a lot like Nim and a variant of the subtraction game (mentioned later in the article). The subtraction game in particular has a really nice and cheesy solution. The major difference is that you prevent taking any number away twice.

On to the code!

Default values

When you start a game, the first player to go is, well, the first player. I don't think a user really wants to waste their time typing it. A similar argument goes for current_sum. You shouldn't really need to specify it. Hide it make the default value 0. so:

def try_best_win(numbers, flags, expected_sum, current_sum=0, is_first_player=True):

Is something you should probably do.

You don't really need flags.

You have a list of numbers (which btw, is a generalization of your problem statement, why?). Each one of these numbers represents a move you cannot play again. So in the recursive call:

Apply the move
Remove the move from numbers
Recurse on the new set of numbers.

This is equivalent to what you are doing and removes the flag variable entirely.

Bugs

There are bugs in your program. @Mathias provides an example as to why your code is wrong. You should consider @vnp's comment. The typical game tree traversal involves a reduction phase of some sort. As another hint, you should really review slide 27/39 of these slides. (Ask yourself, what does your code do differently than what is on that slide!)

Abstraction (and more hints)

You can actually abstract a lot of this. See my question here for a general framework on solving these games. A general framework declutters a lot of code. However, as mentioned in the prelude, a mathematical analysis may (or may not) provide a better solution than the general framework.