Adjusting all depth data in a file by some elevation The code adds up numbers from two different text files. It opens an input file 'DEPTH.dat' and reads the numbers after some headers as follows:

SCALAR
ND    3
ST    0
TS    1000
1.0
1.0
1.0
SCALAR
ND    3
ST    0
TS    2000
3.3
3.4
3.5
SCALAR
ND    3
ST    0
TS    3000
1.7
1.8
1.9

It then adds to them numbers which have been read from a smaller text file 'ELEVATION.dat' which looks as follows:

SCALAR
ND    3
ST    0
TS    0
10.0
10.0
10.0

The result is written in a text file 'output.txt' like this:

SCALAR
ND    3
ST    0
TS    1000
11.0
11.0
11.0
SCALAR
ND    3
ST    0
TS    2000
13.3
13.4
13.5
SCALAR
ND    3
ST    0
TS    3000
11.7
11.8
11.9

Here's the code in question.  As I have learned from 200_success in my last code review that I should write the code using functions, I tried to write this new code using some functions.

For a small example, the code works properly but for bigger input data, it does not work. (For a 350 MB file with 30 million lines, there is no response after two hours.)  How can the code be improved?

from itertools import zip_longest

def grouper(iterable, n, padvalue=None):
    return zip_longest(*[iter(iterable)]*n, fillvalue=padvalue)

def writing_ND(f1):
    for line1 in f1:
        if line1.startswith('ND'):
            ND = float(line1.split()[-1])
            return ND

def writing_TS(f):
    with open(f, 'r') as f:
        for line1 in f:
            if line1.startswith('TS'):
                x = float(line1.split()[-1])
                TS.append(x)
        return TS
TS = []
ND = []
n = 0
add_numbers = []
TS = writing_TS('DEPTH.dat')
with open("ELEVATION_LHP.dat") as f, open("DEPTH.dat") as f1,\
     open('output.txt', 'w') as out:
    ND = writing_ND(f)
    n = int(ND)+4
    f.seek(0)
    for lines in grouper(f, int(n)):
        for item in lines[4:]:
            add_numbers.append(float(item))
    i = 0
    for l in grouper(f1, n):
        data_numbers = []
        for line in l[4:]:
            data_numbers.append(float(line.split()[-1].strip()))
            result_numbers = [x + y for x, y in zip(data_numbers, add_numbers)]
        del data_numbers
        out.write('SCALAR\nND    %d\nST  0\nTS      %0.2f\n' % (ND, TS[i]))
        i += 1
        for item in result_numbers:
            out.write('%s\n' % item)

Minor issues

You have some minor funny business going on:

Repurposing f in writing_TS() makes the code harder to follow:

with open(f, 'r') as f:

In writing_TS(), you do

TS.append(x)

… where you are modifying TS, which is a global variable.  But then you return TS as well, which makes it pointless to communicate via a global variable.
On the other hand, you also have a global variable ND, which is initially an empty list, but becomes a float, but you actually want it to be an int.  But why bother initializing n = 0 at all?

Performance

Your ELEVATION.dat is a small file, and your DEPTH.dat is a huge file.  So don't process DEPTH.dat twice!  You want to run through DEPTH.dat linearly, in a single pass.

When performance is a problem, just keep it as simple as possible.  The core of the solution should "ideally" resemble this:

with open('DEPTH.dat') as depth, open('OUTPUT.dat', 'w') as out:
    for line in depth:
        out.write(line)

Of course, that doesn't do anything useful — it just copies the file line by line.  But it gives you the outline of the code to strive for, and it also establishes a lower bound on the running time.  (Try it.  If you aren't satisfied with the performance, then you know that your task cannot be accomplished satisfactorily using Python.)

With that in mind, how can we modify the skeleton to actually do the work?  I would make two assumptions:

DEPTH.dat consists of repeated blocks resembling the structure of ELEVATION.dat.
The lines of DEPTH.dat should be copied verbatim, except the numeric lines, which should be summed with their counterpart in ELEVATION.dat.

Here is how I would express those two ideas in code:

import itertools

def read_elevation(filename):
    """
    Read elevation data.  Lines in the file that consist only of a float are
    converted to a float; all other lines are preserved.
    """
    def try_parse_float(line):
        try:
            return float(line)
        except ValueError:
            return line

    with open(filename) as f:
        return [try_parse_float(line) for line in f]

elevation_cycle = itertools.cycle(read_elevation('ELEVATION_LHP.dat'))
with open('DEPTH.dat') as depth, \
     open('OUTPUT.dat', 'w') as out:
    for depth_line, elev in zip(depth, elevation_cycle):
        if isinstance(elev, float):
            print(elev + float(depth_line), file=out)
        else:
            out.write(depth_line)

This is simpler than your original code, in that I'm not trying as hard to "make sense" of the data by parsing ND.  It's not assuming that there are four header lines per group, namely SCALAR, ND, ST, and TS.  There isn't even any grouping involved: it just blindly assumes that DEPTH.dat contains many repetitions of blocks resembling ELEVATION.dat.


Swapping characters pairs in a string I have to swap the characters in a string before sending said string to a device to show the information sent through a LCD, the method I developed is as follows:

private string StringSwap(string stringToSwap)
{
    if ((stringToSwap.Length % 2).Equals(1))
    {
        stringToSwap += " ";
    }

    char[] array = stringToSwap.ToCharArray();

    for (int i = 0; i < array.Length; i += 2)
    {
        char temp = array[i];
        array[i] = array[i + 1];
        array[i + 1] = temp;
    }
    return new string(array);
}

I want to know if there are any implementations that do this already (I feel like I am reinventing the wheel here) or, in case there is not, know how to optimize and make this code more resilient, while also making it more readable.

Also, should I make this an extension for the string class instead of a method?

I haven't run the code, but are you sure it does as you expect it to for odd length strings? By padding the last character with a space, you get some... interesting results.

For example, inputting "foo" will return "of o". That doesn't seem quite right to me. I would expect it to return "ofo".  I would do something like this.

private string StringSwap(string stringToSwap)
{
    char[] array = stringToSwap.ToCharArray();

    // even size strings iterate the whole array
    // odd size strings stop one short
    int offset = (stringToSwap.Length % 2)
    for (int i = 0; i < array.Length - offset; i += 2)
    {
        char temp = array[i];
        array[i] = array[i + 1];
        array[i + 1] = temp;
    }
    return new String(array);
}

Essentially, when the mod of 2 equals zero, the code behaves exactly like yours does now, but when the mod of 2 equals one, the loop exits one iteration early, avoiding the IndexOutOfRangeException that prompted you to hack in that white space.

Also note that I changed

    return new string(array);

To

     return new String(array);

It's terribly picky and pedantic, but I find it poor form to use the string alias when you're using it as a class. Seeing a ctor called on a camelCased identifier just looks weird to me. Same goes for things like string.Empty. But, like I said, that's just me being nit picky and pedantic. It's not actually a problem.


Project Euler #5 - Lowest Multiple of 1 through 20 I was trying to solve problem 5 of Project Euler, and this was I needed to do:

  2520 is the smallest number that can be divided by each of the numbers
  from 1 to 10 without any remainder.

  What is the smallest positive number that is evenly divisible by all
  of the numbers from 1 to 20?

I came up with a solution, and I get the correct answer in about 7 minutes, but my question is, how could I make it faster? Calculation speeds are incredibly slow, how could I optimize it?

        for (long x = 1; x < 1000000000; x++) {

            bool dividable = true;

            for (int y = 20; y > 0; y--) {
                if (x % y != 0) {
                    dividable = false;
                }

            }
            if (dividable == true) {
                Console.WriteLine (x);
            }
        }
    }

A number of answers have pointed out some algorithmic changes, and some logic steps you can take, to calculate the result, but there's a simpler way to do it, which is not necessarily the fastest way, but it is still blazingly fast, and still very simple.

The algorithm you have by testing every number is obviously overkill, but, what about going through the basics of the process, and implementing it in code. What is the question actually asking? It's asking for the lowest common multiple of all the numbers 1 through 20.

Mathematically, 1 is a multiple of 1. so, we start there...

Then, find the multiple of 1 that is also a multiple of 2... to do that, we add the 1 to itself repeatedly, until we get a multiple of 2. This is easy, it is just once....

Now we have 2, which is a multiple of both 1, and 2. We want to find the multiple of both 1 and 2 (which is 2), that is also a multiple of 3, so, we check whether 2 is a multiple (it's not), so we add 2 to itself, which is 4. Is 4 a multiple? No, so we add 2 again, to get 6. Is 6 a multiple of 3? Yes. So, 6 is the lowest multiple of 1, 2, and 3. Now, we just repeat this process to 20.

Let's introduce some names here, lcm is the lowest common multiple, and sum is some value that is a multiple of lcm. If sum is a multiple of lcm, then it is also a multiple of everything that lcm is a multiple of.

The rest is actually easier to understand with code....., and it can be generalized very easily with a method like:

public static int GetLCM(int from, int to) {
    // lcm is the Lowest Common Multiple, which starts as just 'from'
    var lcm = from;
    for (int i = from; i <= to; i++) {
        var sum = lcm;
        while (sum % i != 0) {
            sum += lcm;
        }
        // sum is now the first multiple of lcm that is also a multiple of i
        lcm = sum;
    }
    return lcm;
}

Now, I have taken that concept and implemented it on Ideone, and you can see that it arrives at the right solution, using a simple algorithm, in almost no time.


Converting javax.security.cert.X509Certificate[] to java.security.cert.X509Certificate[] keeping order I have got an array of javax.security.cert.X509Certificate and need to convert it into an array of java.security.cert.X509Certificate.

My thoughts when I implemented the following code was that I must keep the order of the certificate thus I used a LinkedList. In the first step I am converting all of them into the new data type and appending it to the LinkedList. Afterwards I create temporary array of a size which equals the amount of certificates in LinkedList. Iterating over the LinkedList allows me to put each single certificate into the temp array and finally I can set my my private java.security.cert.X509Certificate array field. I have got the feeling that it is unnecessarily verbose and not efficient and could be shorter.

private X509Certificate[] x509Certificates;

private void setX509Certificates(javax.security.cert.X509Certificate[] certs)
{
  LinkedList<X509Certificate> x509CertificateLinkedList = new LinkedList<>();

  try
  {
    CertificateFactory certificateFactory = CertificateFactory.getInstance("X.509");

    for (javax.security.cert.X509Certificate cert : certs)
    {
      try
      {
        ByteArrayInputStream bais = new ByteArrayInputStream(cert.getEncoded());
        X509Certificate x509Certificate = (X509Certificate) certificateFactory.generateCertificate(bais);
        x509CertificateLinkedList.add(x509Certificate);
      }
      catch (CertificateEncodingException e)
      {
        e.printStackTrace();
      }
    }

    X509Certificate[] tempX509Certificates = new X509Certificate[x509CertificateLinkedList.size()];

    for (int i = 0; i < x509CertificateLinkedList.toArray().length; i++)
    {
      if (x509CertificateLinkedList.get(i) != null)
      {
        tempX509Certificates[i] = x509CertificateLinkedList.get(i);
      }
    }

    this.x509Certificates = tempX509Certificates;
  }
  catch (CertificateException e)
  {
    e.printStackTrace();
  }
}

private X509Certificate[] x509Certificates;

private void setX509Certificates(javax.security.cert.X509Certificate[] certs) throws CertificateException {
    CertificateFactory certificateFactory = certificateFactory = CertificateFactory.getInstance("X.509");
    List<X509Certificate> x509List = new ArrayList<>();

    for (javax.security.cert.X509Certificate cert : certs) {
        try {
            ByteArrayInputStream bais = new ByteArrayInputStream(cert.getEncoded());
            X509Certificate x509Certificate = (X509Certificate) certificateFactory.generateCertificate(bais);
            if (x509Certificate != null) {
                x509List.add(x509Certificate);
            }
        } catch (CertificateEncodingException e) {
            e.printStackTrace(System.err);
        }
    }
    this.x509Certificates = x509List.toArray(new X509Certificate[]{});
}

Linked List which you have used for temporary purpose. This could be achieved in a single for loop. Since there will not be any duplicate certificate it is safe to use ArrayList for collecting the certifcates.

I am not sure your exception handling strategy. So I am throwing the CertificateException you are free to change it based on your exception strategy.


Convert minutes portion of time to decimal I am making an hours calculator app. It takes a start time, end time and time taken for lunch.
For the start and end time it takes a four-digit hh:mm time. For example: 10:20.
I have made a function which converts the time into a decimal,
so 10:20 = 10.33. The function works, but I feel it looks a little heavy and wondered if anyone has any suggestions of how I could make it better...
const minuteConverter = time => {
    let h = Number(time.split(':')[0]);
    let m = Math.round((1 / 60 * (Number(time.split(':')[1])) + Number.EPSILON) * 100) / 100;
    let mConverted = Number(m.toString().split('.')[1])
    return Number(`${h}.${mConverted}`)
};
console.log(minuteConverter('10:20'))

The time must be output as a number to two decimal places. For example,

'10:20' >> 10.33
'9:45' >> 9.75
'15:33' >> 15.55

Use const instead of let to declare variables if the value doesn't change.

You are executing time.split(':') twice.

A short method to convert a string to a number is the unary +.

JavaScript has the toFixed() method to format a number to a fixed number of digits:

function minuteConverter(time) {
   const [h, m] = time.split(':');
   const value = +h + +m / 60;
   return value.toFixed(2);
}


Skip_over() algorithm I'm just curious if this is clear to the average person.

template<typename IteratorType>
inline IteratorType skip_over(
    IteratorType begin,
    IteratorType end,
    typename std::iterator_traits<IteratorType>::value_type skippedCharacter)
{
    typedef typename std::iterator_traits<IteratorType>::value_type value_type;
    return std::find_if(begin, end,
            std::not1(
                 std::bind2nd(std::equal_to<value_type>(), skippedCharacter)
            )
        );
}

A few comments:

Algorithms typically use the names first and last for the iterators they take, not begin and end (in common usage, begin and end refer specifically to the iterators that delimit a range in a container).
The use of find_if seems a bit excessive:  yes, it is good to use the Standard Library algorithms, but if you are writing your own algorithm, you may as well just write a loop, especially if it makes the code much clearer.
With respect to template parameter naming, it is helpful if you say what category of iterator is required; this helps to document the algorithm.

Consider the following, alternative implementation:

template <typename ForwardIterator, typename T>
ForwardIterator skip_over(ForwardIterator first, ForwardIterator last, T const& x)
{
    while (first != last && *first == x)
        ++first;

    return first;
}


Convert a string into a dictionary I have a string like "food | dairy | milk". The string may consist of more or less words. How do I turn it into a dictionary {'food': {'dairy': {'milk': {'eco': {}}}}}?

The only way I could achieve it was to create a string and then turn it into a dictionary using exec. I understand it's messy. How could I improve on it?

#!/usr/bin/env python -tt
# -*- coding: utf-8 -*-
# I have a string that represents 'path' to place where new item should be inserted
st="food | dairy | milk"
print st,"< -- string"
keys=st.split(" | ")
print keys,"<-- keys"

new_cat="eco" # should be added
d = {i:{} for i in keys}
print d,"d"

ex_str="{"
for i in keys:
    ex_str+="\""+i+"\":{"
ex_str+="\""+str(new_cat)+"\":{}"+"}"*(len(keys)+1)
exec"nd="+ex_str
print nd,"<-- new dict"

You use a temporary variable:

st="food | dairy | milk"
keys=st.split(" | ")

new_cat="eco" # should be added
d = t = {} # t is my temporary dictionary

for i in keys:
    t[i] = {}
    t = t[i]
t[new_cat] = {}

print d


An expirable LRU cache implementation I implemented an ExpireLRUCache class, which will clear the data when it times out. There are two ways to achieve this:

Use a timer to clear the expire data
Call the clean function in Add and Get

If I use a struct in class, and it use the template typename, how to deal with this?
I put the declaration of Node at the top, because it will be used when it is used. Is it reasonable?
template <typename K, typename V>
class ExpireLRUCache {
 private:
  using Timestamp = std::chrono::time_point<std::chrono::system_clock>;
  struct Node {
    K key;
    V value;
    Timestamp timestamp;
  };

 public:
  using NodePtr = std::shared_ptr<Node>;
  using NodeIter = typename std::list<NodePtr>::iterator;
  using ExpiredCallBack = std::function<void(K, V)>;

  // Default timeout is 3000ms.
  ExpireLRUCache(size_t max_size)
     : max_size_(max_size), time_out_(3000), expired_callback_(nullptr) {}

  ExpireLRUCache(size_t max_size, uint32_t time_out, ExpiredCallBack call_back)
     : max_size_(max_size), time_out_(time_out), expired_callback_(call_back) {}

  void Add(K key, V value);
  V Get(K key);

  size_t Size() const;

 private:
  void Expired();

  mutable std::mutex mutex_;
  std::list<NodePtr> list_;
  std::unordered_map<K, NodeIter> map_;

  size_t max_size_;
  // ms
  uint32_t time_out_;

  ExpiredCallBack expired_callback_;
};

template <typename K, typename V>
void ExpireLRUCache<K, V>::Add(K key, V value) {
  std::lock_guard<std::mutex> lock(mutex_);
  // if full, delete oldest
  if (list_.size() >= max_size_) {
    auto oldest = list_.back();
    list_.pop_back();
    map_.erase(oldest->key);
  }

  // if exist, delete it in the list, and then add to the front
  // then overwrite in map.
  if (map_.find(key) != map_.end()) {
    NodeIter iter = map_[key];
    list_.erase(iter);
  }

  auto timestamp = std::chrono::system_clock::now();
  NodePtr node = std::make_shared<Node>(Node{key, value, timestamp});
  list_.push_front(node);
  map_[key] = list_.begin();
}

template <typename K, typename V>
V ExpireLRUCache<K, V>::Get(K key) {
  std::lock_guard<std::mutex> lock(mutex_);

  // Todo(zero): how to call
  Expired();

  if (map_.find(key) != map_.end()) {
    return (*map_[key])->value;
  } else {
    return V{};
  }
}

template <typename K, typename V>
void ExpireLRUCache<K, V>::Expired() {
  auto time_now = std::chrono::system_clock::now();

  while( !list_.empty() ) {
    auto oldest = list_.back();
    auto diff = std::chrono::duration_cast<std::chrono::milliseconds>(
                    time_now - oldest->timestamp);
    if (diff.count() > time_out_) {
      list_.pop_back();
      map_.erase(oldest->key);
      expired_callback_(oldest->key, oldest->value);
    } else {
      break;
    }
  }
}

template <typename K, typename V>
size_t ExpireLRUCache<K, V>::Size() const {
  std::lock_guard<std::mutex> lock(mutex_);
  return map_.size();
}
```

I don't see why the typenames NodePtr and NodeIter are public.  Node itself is private, and none of the public interface uses these two types, so I think they should be private, too.

I'm not sure I like this interface:
V Get(K key);

This means we can only cache value types.  Not only that, but there's no way to indicate "not in the cache"; I see the implementation default-constructs a return value in that case - further limiting the types that can be used.

Expired() expects that the mutex is held, but is missing a comment stating that, so it took too long to confirm that it's used correctly.

What's going on here?

NodePtr node = std::make_shared<Node>(Node{key, value, timestamp});

Why construct a Node and pass it to make_shared() to copy?  It would be better to make_shared() directly:
NodePtr node = std::make_shared<Node>(key, value, timestamp);


Calculating the sum of the digits in 2^1000 I have a program which calculates the sum of the digits in the number 21000. The program is not beautiful and I was hoping for some advice on how to improve it.

The idea I have used is to create an array with the first element equal to 20. The first element in the array represent 100 the second 101 and so on.
Then go through and double every element in the array. After every element is doubled I check if an element in the array is greater than 9, if it is I subtract 10 and add a 1 to the next element in the array. For example if the element at place 40 is equal to 13 and element at place 41 is equal to 3 then I will change the element at place 40 to 3 and element at place 41 to 4.

#include <stdio.h>
int main(void){
 int sum = 0;
 int dec[320] = {1, };
 int i,j,p,q;
 for(i = 1; i < 1001; i++){

   for (j = 0; j < 302; j++) {
     dec[j] = 2*dec[j];
     }

     for(q = 0; q < 302; q++){
       if(dec[q] > 9){
         dec[q] = dec[q] % 10;
         dec[q+1] = dec[q+1] + 1;
     }
   }

 }

 for(p = 0; p < 302; p++){
   sum += dec[p];
 }
 printf("sum = %d sista %d \n", sum, dec[301]);
}

I see some things that may help you improve your code.

Eliminate "magic numbers"

The code, is full of "magic numbers" -- that is, raw numbers in the code that don't have obvious meaning.  For example, we have this:

int dec[320] = {1, };
// and later ...
for (j = 0; j < 302; j++) {

It's unclear, whether 302 and 320 is just a typo or has some signficance.  Better would be to use a #define for this.  I'd use this:

#define BITS 1000
#define DIGITS 302

Write in idiomatic C

Lines like these are a little peculiar to an experienced C programmer:

dec[q] = dec[q] % 10;
dec[q + 1] = dec[q + 1] + 1;

The idiom instead would be to write them like this:

dec[q] %= 10;
++dec[q + 1];

Avoid potentially expensive operations

Since you're only doubling each digit each pass, it's certain that the maximum possible value is 9*2 = 18, so rather than using the % operator, it's sufficient to simply subtract 10.

Make only a single pass through

There's no need to make two passes through the array for each doubling.  It can be done in a single pass like this:

for (int i = BITS; i; --i) {
    int carry = 0;
    for (int j = 0; j < DIGITS; j++) {
        dec[j] = 2 * dec[j] + carry;
        if (dec[j] > 9) {
            dec[j] -= 10;
            carry = 1;
        } else {
            carry = 0;
        }
    }


I think your hexdump implementation looks pretty good.  I have no immediate comments on the implementation.  I will however comment on the implied requirements.

Hex Dumper Definition

Most hex dumpers that I am familiar with dump hex as a modulo of the stride length.  The example you show implies that, but that is because your example uses n=32, where 32 is an even modulus of the stride length (16).  If you pass in different stride lengths, or pass in an n that is not an even modulus of the stride, the output doesn't (to my eye) look quite as nice.

So I suggest you consider adding another parameter (let's call it base_addr) which is the address of the beginning of the byte array.  And, then also consider adding fill at the beginning of the dump to allow it to align the dump with an even modulus of the stride length.  Such that:

hexdump_gen(byte_string, base_addr=1, n=1, sep='')

Would produce:

00000000        9a 39 81 c2 b5 b9  ce 02 97 39 b5 19 a0 b9  |  .9.......9....|
00000010  ca 02 97 39 b5 19 a0 b9  ca 02 97 39 b5 19 a0 b9  |...9.......9....|
00000020  ca 8c 96 39 fb 89 8e b9  0a 6a b1 39 81 18 84 b9  |...9.....j.9....|
00000030  95 6a b1 39 81 18 84 b9  95 6a b1 39 81 18 84 b9  |.j.9.....j.9....|
00000040  95 6a b1 39 81 18 84 b9  95 6a b1 39 81 18 84 b9  |.j.9.....j.9....|
00000050  95                                                |.               |

One way that could be done:

def hexdump_gen(byte_string, _len=16, base_addr=0, n=0, sep='-'):
    not_shown = ['  ']
    leader = (base_addr + n) % _len
    next_n = n + _len - leader
    while byte_string[n:]:
        col0 = format(n + base_addr - leader, '08x')
        col1 = not_shown * leader
        col2 = ' ' * leader
        leader = 0
        for i in bytearray(byte_string[n:next_n]):
            col1 += [format(i, '02x')]
            col2 += chr(i) if 31 < i < 127 else '.'
        trailer = _len - len(col1)
        if trailer:
            col1 += not_shown * trailer
            col2 += ' ' * trailer
        col1.insert(_len // 2, sep)
        yield FMT.format(col0, ' '.join(col1), col2)
        n = next_n
        next_n += _len

Symmetric parameters

The n parameter is an offset into the bytearray, which specifies where in the bytearray to start the dump.  But there is no equivalent end address.

So currently, the dumper always goes to the end of the byte array.  From a symmetry perspective it would seem a good idea to also provide a terminal condition.


Building Data abstraction for line segments using "objects" I follow this definition of "object":An object is a value exporting a procedural interface to data or behavior. Objects use procedural abstraction for information hiding, not type abstraction. Object and and their types are often recursive. Objects provide a simple and powerful form of data abstraction. They can be understood as closures, first-class modules, records of functions, or processes. Objects can also be used for procedural abstraction. from this paper
Below is the given exercise:

Exercise 5: Segments
Consider the problem of representing line segments in a plane. Each segment is represented as a pair of points: a starting point and an ending point. Define a constructor make-segment and selectors start-segment and end-segment that define the representation of segments in terms of points. Furthermore, a point can be represented as a pair of numbers: the x coordinate and the y coordinate. Accordingly, specify a constructor make-point and selectors x-point and y-point that define this representation. Finally, using your selectors and constructors, define a procedure midpoint-segment that takes a line segment as argument and returns its midpoint (the point whose coordinates are the average of the coordinates of the endpoints)

It is taught in class that data abstraction is the methodology to create barrier between "how data values are used" and "how data values are represented". Also, an abstract data type is some collection of selectors and constructors, together with some behaviour conditions (invariants).
It is compound data that needs data processing which actually enables to think about data abstraction because the user would like to use this compound data as single unit.
Below code is building "data abstraction" and "ADT" for "line segments" using python objects
# Representation - start
#Constructor
def make_point(x, y):
    return (x, y)

#Selector
def x_coordinate(point):
    return point[0]

#Selector
def y_coordinate(point):
    return point[1]

#Constructor
def make_segment(point1, point2):
    return (point1, point2)

#Selector
def start_segment(lineSegment):
    return lineSegment[0]

#Selector
def end_segment(lineSegment):
    return lineSegment[1]

#Representation - end

#Use -start
def midpoint_segment(lineSegment):
    return make_point((x_coordinate(start_segment(lineSegment)) + x_coordinate(end_segment(lineSegment)))/2, (y_coordinate(start_segment(lineSegment)) + y_coordinate(end_segment(lineSegment)))/2)

#Use - end

#Driver code from user
p1 = make_point(1,2)
p2 = make_point(3, 4)
line = make_segment(p1, p2)
midpoint = midpoint_segment(line)
print(midpoint)

In the above code, constructor and selectors constitute ADT.
In the above two implementations:

There is an abstract data type that supports an invariant1:
If we construct point p from x-coordinate a and y-coordinate b, then x_coordinate(p), y_coordinate(p)  must equal a, b

There is an abstract data type that supports an invariant2:
If we construct a line segment l from point p1 and point p2, then start_segment(l)______end_segment(l) must equal p1______p2

In the above implementation:

Parts of the program that use line segments to perform computation use midpoint_segment.

Parts of the program that implement midpoint_segment use constructor and selectors only

Parts of the program that create line segments use constructor make_segment

Parts of the program that implement constructor for line segment use tuple

Parts of the program that implement selectors for line segments use [] for indexing tuples.

So, above implementation has set of "objects" that build "Data abstraction"
Is my understanding correct on designing data abstraction and ADT for line segments?

Although your question indicates that you are using a functional paradigm, you are not. You are using a procedural paradigm.

Functional programming is about declaring functions, and then passing those functions in to other places to be used as modifiers and manipulators for other processes. Terms like 'lambdas', and 'closures', 'optionals' or 'monads' are required.

Procedural programming is the act of defining a process to follow, sometimes returning a value, but "I take this data and process it following this defined routine, and I get the results I want."

Procedural programming often has functions declared, but the functions are not used as parameters to other functions.

You do not pass any functions in as parameters, you are not using a "functional paradigm". You are using a "procedural paradigm".

See Wikipedia's comparison between Functional and Procedural programming.

Your code is a very poor implementation of a Functional paradigm.

On the other hand, as a Procedural implementation, it is actually quite good, except for the get_midpoint method, which is implemented on a single line which is too long, and makes it hard to read with all the nested braces on one line.

def midpoint_segment(lineSegment):
    start = start_segment(lineSegment);
    end = end_segment(lineSegment)
    xsums = x_coordinate(start) + x_coordinate(end)
    ysums = y_coordinate(start) + y_coordinate(end)
    return make_point(xsums / 2, ysums / 2)


ASP.NET controls positioning I haven't studied a lot about positioning of controls in ASP.NET using .Net framework 3.5 but still I know following control positioning technique isn't the best one, just to mention its a user control,

    <asp:Label ID="Label2" runat="server" Text="Number of Days"></asp:Label>
&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;
    <asp:TextBox ID="TextBox2" runat="server"></asp:TextBox>

Right now I am using tabs and spaces to position controls but can I do it in a better way.

Yes, use CSS and some HTML to put things where they belong. Theres is nothing special about ASP.NET controls just check the generated HTML markup on the client side.


ANSI colors cross platform I am trying to write for both NT and POSIX (my two usual platforms) and have come up with the code below. The clrs code I refer to is apparently from blender build scripts, I discovered it on this SO post. They called the class bcolors but I wanted to reduce clutter so I renamed it.

But the main class is what I've written that determines the os before printing. Prints are called, for example, via PRN.cyan("some string").

For this to work I import colorama:

from colorama import init, Fore, Back, Style
init()

The PRN class that handles the print calls:

class PRN:
    @staticmethod
    def cyan(p_str):
        """print some text with cyan foreground"""
        if os.name == 'nt':
            # we'll try the Colorama option but if it fails we can
            # change the global variable for colours.
            if coloursOn:
                sys.stdout.write(Fore.CYAN + p_str + Fore.RESET)
            else:
                sys.stdout.write(p_str)
        else:
            # try Deez1.clrs for posix environment
            sys.stdout.write(clrs.cyan + p_str + clrs.ENDC)

If colorama doesn't work on a Windows system for some reason I can change the global boolean coloursOn.

Is this an incredibly klutzy/dumb implementation? From what I've read, so far, cross platform capability for ANSI color printing has only gotten shakier, not simpler.

First off, "cross platform capability for ANSI color printing" doesn't quite make sense (if I understand correctly). ANSI color printing is only going to work on terminals that are ANSI compatible.

Therefore, it doesn't really matter what OS it is: if it's an ANSI terminal, you can use ANSI colored output.

Therefore, you can remove the checking of the operating system.

Note: according to this answer,

  "All modern terminal emulators use ANSI escape codes to show colours
  and other things."

Judging by the name of the method your presented and the context of the code, you have created a separate method for each color.

This sounds like you are repeating yourself a lot.
It's not very flexible for the code that is using your library.

I recommend that you create a single method print_color_string that takes a string and the color to print.

def print_color_string(str, color):
    ansi_color = getattr(Fore, color)
    if ansi_color:
        sys.stdout.write(ansi_color + p_str + Fore.RESET)
    else:
        # throw error because color is not a valid color


Mailinator automatic checker for new e-mails I'm making a little script to run on the background and notify me when I received a new email in a mailinator inbox. I want to eventually daemonise it, but for now it just runs like a regular script.

Could you give me some pointers about the code itself, the code structure and so on? I'm not sure if I should have my functions in a particular order, they're quite randomly scattered around the script at the moment.

#!/usr/bin/env python3
#encoding: utf-8

import time
import argparse
import requests

from IPython import embed as qq

try:
    from apikey import API
except ImportError:
    API = input('Please provide your API key ')

CHECKING_FREQ = 30

def parse_arguments():
    parser = argparse.ArgumentParser(description="Mailinator inbox checker")
    parser.add_argument('inbox', nargs=1, help='The inbox that you want to check')
    return parser.parse_args()

def is_new(emails):
    # Here I could probably use any() instead of next()
    # but I'm thinking I might want the email itself later on
    return next((email for email in emails
                 if email['seconds_ago'] <= CHECKING_FREQ),
                False)

def check_inbox(inbox):
    inbox_page = get_inbox(inbox)
    return inbox_page['messages']

def wait_email(inbox):
    while 1:
        emails = check_inbox(inbox)
        if is_new(emails):
            print('New email received.')
        time.sleep(CHECKING_FREQ)

def get_inbox(inbox):
    return requests.get('https://api.mailinator.com/api/inbox?'
                        'to={inbox}&token={api}'.format(
                            inbox=inbox,
                            api=API)).json()

def start_daemon():
    ...

def main():
    args = parse_arguments()
    start_daemon()
    wait_email(args.inbox[0])

if __name__ == '__main__':
    main()

Overall it looks great, but here are a couple of thoughts:

print('New email received.'): maybe print a snippet form the message or just the time stamp so you have some idea on when the new email arrived (instead of just having a stream of same strings stating "New email received.").
I'd consider moving the inbox operations into its own class. Its __init__ could be __init__(inbox, api): .... CHECKING_FREQ could be a keyword argument to this, defaulting to 30, but configurable through the input params when running the script. The class could have a fetch_inbox (merging your get_inbox and check_inbox). And finally the has_new_items (just rename your is_new method).
Your wait_email is what I'd call check_inbox because for checking the inbox you first have to fetch the inbox and then check the message timestamps on what was returned.
I feel someone might have a good suggestion on how to do that infinite loop, but I can't think of a good way right now. All I know is it feels a little odd (or maybe it's fine :).

Hopefully this helps.


can this php function be written to perform better? I have this function takes a long time to complete. Is there a way i can improve it to quicken the procedure

function do_updatebonus() {
    global $site_config;

    $res200 = SQL_Query_exec("SELECT DISTINCT userid FROM peers WHERE seeder = 'yes'");

    while ($row200 = mysql_fetch_assoc($res200)) {
        $userid = $row200["userid"];
        $res201 = SQL_Query_exec("SELECT COUNT(torrent) FROM peers WHERE seeder = 'yes' AND userid = $userid");

        $c = mysql_result($res201, 0);
        if ($c >= 5) {
            SQL_Query_exec("UPDATE users SET seedbonus = seedbonus + '" . $site_config["bonuspertime"] . "' WHERE id = $userid");
        }
    }
}

Yeah, you can optimize your query,

 $res201 = SQL_Query_exec("SELECT COUNT(torrent) FROM peers WHERE seeder = 'yes' AND userid = $userid");

As this query will first check for seeder which has 'yes' and then for userid. So you can first check for userid, then for seeder. This will improve performance

$res201 = SQL_Query_exec("SELECT COUNT(torrent) FROM peers WHERE userid = $userid" AND seeder = 'yes' );


E-commerce database design I am new to database design but am fairly familiar with SQL and its syntax. I want to create a database for an e-commerce website that will sell a single product type such as a shirt. They could be different sizes, colors, and design. I formulated list of information I thought would be necessary to collect from a visitor and are included in the tables I made.

Right now I have tables being:

Products
Customer
Credit_Card_Details
Order_Details
State_tax
Shopping_Cart
Purchase_history
Shipping type

The tables I created:

 CREATE TABLE `shipping_type`
  (
 `type_of_shipping` VARCHAR(30) NOT NULL DEFAULT '',
 `price`            DOUBLE(6, 2) NOT NULL,
 `aprox_delivery`   INT(11) NOT NULL,
 PRIMARY KEY (`type_of_shipping`)
 )
 engine=innodb
 DEFAULT charset=latin1;

 CREATE TABLE `shopping_cart`
  (
 `shopping_cart_id` INT(11) UNSIGNED NOT NULL auto_increment,
 `inventory_id`     VARCHAR(11) NOT NULL DEFAULT '',
 `price`            DOUBLE(6, 2) NOT NULL,
 `date`             DATE NOT NULL,
 `user_id`          VARCHAR(30) NOT NULL DEFAULT '',
 `quantity`         INT(11) NOT NULL,
 PRIMARY KEY (`shopping_cart_id`)
 )
 engine=innodb
 DEFAULT charset=latin1;

 CREATE TABLE `state_tax`
 (
 `state_name`     VARCHAR(11) NOT NULL DEFAULT '',
 `sales_tax_rate` DOUBLE(6, 2) NOT NULL,
 PRIMARY KEY (`state_name`)
 )
 engine=innodb
 DEFAULT charset=latin1;

 CREATE TABLE `purchase_history`
 (
 `user_id`      VARCHAR(30) NOT NULL DEFAULT '',
 `inventory_id` INT(11) NOT NULL,
 `date_ordered` DATE NOT NULL,
 `order_id`     INT(11) NOT NULL,
 `quantity`     INT(11) NOT NULL,
 `price`        DOUBLE(6, 2) NOT NULL,
 PRIMARY KEY (`user_id`)
 )
 engine=innodb
 DEFAULT charset=latin1;

CREATE TABLE `products`
(
 `inventory_id` INT(11) UNSIGNED NOT NULL auto_increment,
 `shirt_name`   VARCHAR(50) NOT NULL DEFAULT '',
 `shirt_size`   VARCHAR(30) NOT NULL DEFAULT '',
 `shirt_color`  INT(11) NOT NULL,
 `price`        DOUBLE(6, 2) NOT NULL,
 PRIMARY KEY (`inventory_id`)
)
engine=innodb
DEFAULT charset=latin1;

 CREATE TABLE `order_details`
 (
 `order_id`         INT(11) UNSIGNED NOT NULL auto_increment,
 `user_id`          VARCHAR(30) NOT NULL DEFAULT '',
 `reciever_name`    VARCHAR(100) NOT NULL DEFAULT '',
 `address`          VARCHAR(30) NOT NULL DEFAULT '',
 `city`             VARCHAR(30) NOT NULL DEFAULT '',
 `zip`             VARCHAR(11) NOT NULL DEFAULT '',
 `state`            VARCHAR(30) NOT NULL DEFAULT '',
 `type_of_shipping` VARCHAR(30) NOT NULL DEFAULT '',
 `date_ordered`     DATE NOT NULL,
 PRIMARY KEY (`order_id`)
 )
 engine=innodb
 DEFAULT charset=latin1;

CREATE TABLE `customer`
 (
 `user_id`      VARCHAR(30) NOT NULL DEFAULT '',
 `password`     VARCHAR(30) NOT NULL DEFAULT '',
 `fname`        VARCHAR(30) NOT NULL DEFAULT '',
 `lname`        VARCHAR(30) NOT NULL DEFAULT '',
 `email`        VARCHAR(30) NOT NULL DEFAULT '',
 `address`      VARCHAR(30) NOT NULL DEFAULT '',
 `city`         VARCHAR(30) NOT NULL DEFAULT '',
 `zip`          INT(11) NOT NULL,
 `state`        VARCHAR(30) NOT NULL DEFAULT '',
 `phone number` VARCHAR(11) DEFAULT NULL,
 PRIMARY KEY (`user_id`)
)
engine=innodb
DEFAULT charset=latin1;

CREATE TABLE `credit_card_details`
(
 `cc_num`    VARCHAR(30) NOT NULL DEFAULT '',
 `card_type` VARCHAR(30) NOT NULL DEFAULT '',
 `sec_code`  INT(3) NOT NULL,
 `exp_date`  DATE NOT NULL,
 `user_id`   VARCHAR(30) NOT NULL DEFAULT '',
 PRIMARY KEY (`cc_num`)
)
engine=innodb
DEFAULT charset=latin1;

Is this the most efficient way to do this? I am pretty sure all tables would be in third normal form, and all can be linked. If this is not the correct way to do this, could somebody please point out to me what I could be doing differently to improve my design or how to implement this design in a better way?

Never assume that you're only going to have one product. That will end up being a limiting factor of your design and can cause you a lot of pain later down the line and rules out any kind of code re-usability. Make your products table generic enough to handle any kind of product. Create color/size attribute tables that you can tie to the product if you need to.
What about handling the case where people want to order as a guest or will you rule that out entirely?
How will you handle returned items/payment credits, cancelled orders, or an order where the credit card is declined? Are you going to keep history for those?
You will probably need more than 1 line for address. Are you going to handle international orders? If so you'll need to allow for non-numerical zip codes and ones that don't tie to a state.
If you tie shopping carts to cookies then you can reload someone's shopping cart when they come back based on their cookie's ID.
If you're going to keep credit card numbers in the database then you need to be PCI Compliant (https://www.pcisecuritystandards.org/).


Doubly linked list reimplemented with smart pointers I need some suggestions on my reimplementation of a doubly linked list with smart pointers since I'm new to smart pointers in C++.

I read some docs on smart pointers and found out there are shared_ptr<Obj>, unique_ptr<Obj> and weak_ptr<Obj>. I only use shared_ptr<Obj> in my current code, but I'm not sure how to use unique_ptr<Obj> in my doubly linked list. I would like someone can give me some pointers on how and where to use unique_ptr<Obj> in my DLL.

class Node{
    public:
        int data;
        Node(int data){
            this->data = data;
        }
    public:
        shared_ptr<Node> prev;
        shared_ptr<Node> next;
};

class LinkedList{
    public:
        shared_ptr<Node> head;
        shared_ptr<Node> tail;
    public:
        LinkedList(){}

        void fun(){
            printf("fun()\n");
        }

        void append(shared_ptr<Node> node){
            if(!head){
                head = node;
                tail = head;
            }else{
                tail->next = node;
                node->prev = tail;
                tail = node;
            }
        }

        void print(){
            shared_ptr<Node> curr = head;
            while(curr){
                printf("[%d]\n", curr->data);
                curr = curr->next;
            }
        }
        void remove(shared_ptr<Node> node){
            shared_ptr<Node> curr = head;
            if(node && curr){
                while(curr){
                    if(curr == node){
                        if(curr->prev == nullptr){
                            if(curr->next == nullptr){
                                // [curr]
                                node.reset();
                                head.reset();
                                tail.reset();
                            }else{
                                // [curr]->[]
                                shared_ptr<Node> next = curr->next;
                                curr->next.reset();
                                next->prev.reset();
                                head = next;
                            }
                        }else{
                            if(curr->next == nullptr){
                                // []->[curr]
                                shared_ptr<Node> prev = curr->prev;
                                curr->prev.reset();
                                prev->next.reset();
                                tail = prev;
                            }else{
                                // []->[curr]->[]
                                shared_ptr<Node> next = curr->next;
                                shared_ptr<Node> prev = curr->prev;
                                next->prev = prev;
                                prev->next = next;
                            }
                        }
                    }
                    curr = curr->next;
                }
            }
        }
};

Summary

This is obviously a personal opinion, but I don't really like the interface you've defined for your list.  By defining your methods in terms of a shared_ptr<Node>, you're making the caller responsible for constructing Nodes.  It seems like Node should be an implementation detail of your list.  Otherwise, since the caller has complete access to the Node they can iterate the list and modify it without the LinkedList class being called at all.

I'd define the interface in terms of the Data that is to be stored, currently this is an int, so you would have something like this:

void append(int newValue)

Or you may want to define your list in terms of a template and take in a pointer of the relevant type.  This could be a shared_ptr if ownership of the data doesn't pass solely to your list, or it could be a unique_ptr if the node that references it will be the owner of the Data.

Append

Looking at your current implementation, what happens if a Node is passed in that already has its prev and next pointers populated (with the given interface I could assume that I could insert a Node chain).  These pointers aren't checked, they're overwritten.  This is part of why I don't like the interface...

Remove

What happens if you're asked to remove a node that isn't in the list?  Currently nothing.  It feels like this method should either be returning an indicator that the object has been removed, or more likely throwing an exception to indicate that it hasn't..


Static ConcurrentDictionary to maintain static objects I am using following code to maintain some static information. The problem I see with it is that, if the information retrieved using GeKeysFromCache is modified without using lock keyword it may lead to exceptions in a multithreaded environment. Is there a way to improve this implementation?

public class CacheHelper
{
    private static ConcurrentDictionary<string, List<InstanceField>> m_InstancesCached = new ConcurrentDictionary<string, List<InstanceField>>();
    private static readonly Object _entriesLock = new object();

    public List<InstanceField> GeKeysFromCache(string cacheKey)
    {
        return m_InstancesCached.GetOrAdd(cacheKey, (key) =>
        {
            return new List<InstanceField>();
        });
    }

    public void AddKeysToCache(string cacheKey, List<InstanceField> inputs)
    {
        m_InstancesCached.AddOrUpdate(cacheKey, inputs, (key, oldValue) =>
        {
            lock (_entriesLock)
            {
                oldValue.AddRange(inputs);
                return oldValue;
            }
        });
    }
}

Test Code: Without lock keyword it fails

public class ThreadSafeCachingTest
{
    private static readonly Object _entriesLock = new object();
    public ThreadSafeCachingTest()
    {
    }

    public void Invoke()
    {
        var thread1Completed = false;
        var thread2Completed = false;
        var thread3Completed = false;
        var threadList = new List<Thread>();

        var instanceName = "MyInstance";
        var beforeCachingData = (new CacheHelper()).GeKeysFromCache(instanceName);

        threadList.Add(new Thread(() =>
        {
            //lock (_entriesLock)
            //{
            var instanceFields1 = (new CacheHelper()).GeKeysFromCache(instanceName);
            instanceFields1 = instanceFields1 ?? new List<InstanceField>();

            for (int i = 0; i < 10000; i++)
            {
                instanceFields1.Add(new InstanceField() { FieldName = "FieldName-" + i.ToString() });
            }
            thread1Completed = true;
            //}
        }));

        threadList.Add(new Thread(() =>
        {
            //lock (_entriesLock)
            //{
            var instanceFields2 = (new CacheHelper()).GeKeysFromCache(instanceName);
            instanceFields2 = instanceFields2 ?? new List<InstanceField>();

            for (int i = 10000; i < 20000; i++)
            {
                instanceFields2.Add(new InstanceField() { FieldName = "FieldName-" + i.ToString() });
            }
            thread2Completed = true;
            //}
        }));

        threadList.Add(new Thread(() =>
        {
            //lock (_entriesLock)
            //{
            while (!(thread1Completed && thread2Completed && thread3Completed))
            {
                var instanceFields3 = (new CacheHelper()).GeKeysFromCache(instanceName);
                if (instanceFields3.Count > 0)
                {
                    instanceFields3.RemoveAt(0);
                }
                else
                {
                    thread3Completed = true;
                }
                //}
            }
        }));

        foreach (Thread t in threadList) t.Start();
        foreach (Thread t in threadList) t.Join();

        var afterCachingData = (new CacheHelper()).GeKeysFromCache(instanceName);
    }
}

public class InstanceField
{
    public string FieldName { get; set; }
}

public List<InstanceField> GeKeysFromCache(string cacheKey)
{
    var resultSet = m_InstancesCached.GetOrAdd(cacheKey, (key) =>
   {
       return new List<InstanceField>();
   });

    return resultSet.ToList();
}

Why so many returns and when you can simply do this with only one return and without the lambda:

return m_InstancesCached.GetOrAdd(cacheKey, new List<string>()).ToList();


Extending the functionality of lxml.etree I wrote a class to slightly customize the behavior of lxml.etree.ElementTree and I use it quite extensively. It works great, but there are a few methods that I'm not sure how I wrote, and there are a few other methods that seem awfully redundant. I'll elaborate on specific questions below, but first here's the code.

import lxml.etree as old_etree
from xml_utilities import clean_xml
from datetime import datetime

VERSION = 'X.X.X.X'
TOOL = 'ExampleTool'

class etree(old_etree._ElementTree):
  @staticmethod
  def parse(path):
    old_tdx = old_etree.parse(path)
    new_tdx = etree(old_tdx)
    new_tdx._setroot(old_tdx.getroot())
    return new_tdx

  @staticmethod
  def fromstring(string):
    old_tdx = old_etree.fromstring(string)
    new_tdx = etree(old_tdx.getroottree())
    new_tdx._setroot(old_tdx)
    return new_tdx

  @staticmethod
  def getISOTime():
    iso_time = datetime.now().isoformat()
    format_time = iso_time.split('.')[0] + 'Z'
    return format_time

  @staticmethod
  def SubElement(*args, **kargs):
    return old_etree.SubElement(*args, **kargs)

  @staticmethod
  def Element(*args, **kargs):
    return old_etree.Element(*args, **kargs)

  def getMetaNode(self, name, default=''):
    metas = self.findall('meta')
    for meta in metas:
      if meta.get('name') == name:
        return meta
    return old_etree.SubElement(self.getroot(), 'meta',
                                attrib={'name': name, 'value': default})

  def setMetaNode(self, name, value):
    node = self.getMetaNode(name)
    node.set('value', value)

  def write(self, path, updateTool=True):
    self.setMetaNode('saved', etree.getISOTime())

    if updateTool:
      self.setMetaNode('version', VERSION)
      self.setMetaNode('type', TOOL)

    super(etree, self).write(path)
    clean_xml(path)

The function clean_xml is custom written and largely irrelevant: It escapes troublesome characters that ElementTree.write doesn't by default (this is an issue with the file specification we use, rather than with lxml). For questions:

Is there a better way to extend the functionality of lxml.etree than inheriting from _ElementTree? Ultimately, I want to be changing the behavior of the method ElementTree.write and to add the methods ElementTree.getMetaNode and ElementTree.setMetaNode.
Is there a better way of incorporating the methods parse, fromstring, SubElement, and Element, rather than just creating thin wrappers that reference the original methods?
Does anyone understand what my methods parse and fromstring are doing? I know that my method fromstring has different functionality from lxml.etree.fromstring because I want it to return an ElementTree rather than an Element. These methods were written largely through guess-and-check (oops).
Is there a better way to write the return old_etree.SubElement(...) line so it doesn't have to take up two lines? This isn't major, but...

Originally, I tried to just overwrite lxml.etree.ElementTree.write directly, but that throws the error AttributeError: 'lxml.etree._ElementTree' object attribute 'write' is read-only. General critiques are also welcomed. I know that I should have comments in here, but everything is pretty straight-forward except for the two methods that I don't understand.

Also, I'm not sure if anyone is going to think that using the name etree in order to overwrite lxml.etree is a bad idea. The reason for this is so that I only have to change the import line at the top of any previously written file from from lxml import etree to from my_lxml import etree.

Boilerplate

class etree(old_etree._ElementTree):
  @staticmethod
  def parse(path):
    old_tdx = old_etree.parse(path)
    new_tdx = etree(old_tdx)
    new_tdx._setroot(old_tdx.getroot())
    return new_tdx

  @staticmethod
  def fromstring(string):
    old_tdx = old_etree.fromstring(string)
    new_tdx = etree(old_tdx.getroottree())
    new_tdx._setroot(old_tdx)
    return new_tdx

In those boilerplate functions, I don't know what "tdx" means. I guess it's linked to the file specification you use.

Those functions are the main problem I have with your code. You mixed the lxml.etree module with the lxml.etree.ElementTree class. Those four methods are methods of the module, not the class! I think you should:

have a real etree module
put your current etree class (renamed as ElementTree) in it
add those functions in the module, not the class
fix the return type to return what lxml returns

This would make things way less confusing because it would be closer to how lxml.etree and the standard xml.etree work. Your coworkers would be able to pick your code faster.

You would do from lxml.etree import * and only redefine the functions you want to redefine. Your probably can't expect a perfect compatibility, but at least the basic API would be the same.

I don't have much to say about the implementation of the functions apart from the return type: ugly but I did not find a way to make things better. And yes, it's quite easy to understand them.

  @staticmethod
  def SubElement(*args, **kargs):
    return old_etree.SubElement(*args, **kargs)

  @staticmethod
  def Element(*args, **kargs):
    return old_etree.Element(*args, **kargs)

You no longer would need do to this with my proposal above.

getISOTime

  @staticmethod
  def getISOTime():
    iso_time = datetime.now().isoformat()
    format_time = iso_time.split('.')[0] + 'Z'
    return format_time

Adding the 'Z' means you're saying this is an UTC time, but it's not since you've used datetime.now() instead of datetime.utcnow(). I live in UTC+4, and the format_time you're returning is 4 hours off for me.

getMetaNode

  def getMetaNode(self, name, default=''):
    metas = self.findall('meta')
    for meta in metas:
      if meta.get('name') == name:
        return meta

You can use lxml XPath support here if you want to avoid the loop: meta.xpath("meta[name = $name]", name=name).

Write

  def write(self, path, updateTool=True):
    self.setMetaNode('saved', etree.getISOTime())

    if updateTool:
      self.setMetaNode('version', VERSION)
      self.setMetaNode('type', TOOL)

    super(etree, self).write(path)
    clean_xml(path)

nitpick: Cleaning at the ElementTree would prevent your filesystem to see two different versions. Say at some point you decide to use watchdog, the callback will kick in before you have a chance to run clean_xml, which could cause subtle bugs.

Answering your questions

  Is there a better way to extend the functionality of lxml.etree than inheriting from _ElementTree? Ultimately, I want to be changing the behavior of the method ElementTree.write and to add the methods ElementTree.getMetaNode and ElementTree.setMetaNode.

Since lxml is written in Cython, I think you can't monkeypatch lxml directly anyway, so subclassing is the way to go. lxml could have chosen to make things easier by using __new__ just like numpy does for facilitating ndarray subclassing.

  Is there a better way of incorporating the methods parse, fromstring, SubElement, and Element, rather than just creating thin wrappers that reference the original methods?

See "Boilerplate" above.

  Does anyone understand what my methods parse and fromstring are doing? I know that my method fromstring has different functionality from lxml.etree.fromstring because I want it to return an ElementTree rather than an Element. These methods were written largely through guess-and-check (oops).

Why do you want to get an ElementTree rather than an Element? I think it's a bad idea for two reasons:

It's very easy to get an ElementTree from an Element.
It's already hard enough to reason about the type of object you get when using lxml, if we can't use our knowledge about ElementTree/lxml it's only going to get worse.

  Is there a better way to write the return old_etree.SubElement(...) line so it doesn't have to take up two lines? This isn't major, but...

If anything, I think I would use more than two lines. If you want to stop worrying about such things, I highly recommand yapf.

  Originally, I tried to just overwrite lxml.etree.ElementTree.write directly, but that throws the error AttributeError: 'lxml.etree._ElementTree' object attribute 'write' is read-only. General critiques are also welcomed. I know that I should have comments in here, but everything is pretty straight-forward except for the two methods that I don't understand.

Yep, you can't monkeypatch it (as said in answer to 1.), but that's not what you wanted to do, since you wanted to keep the original implementation too. And a decorator would not have improved things.

Naming

  Also, I'm not sure if anyone is going to think that using the name etree in order to overwrite lxml.etree is a bad idea. The reason for this is so that I only have to change the import line at the top of any previously written file from from lxml import etree to from my_lxml import etree.

Nope, I think using the name etree is a good idea. It's a common way to provide replacements in Python, and this is way the standard library xml.etree does things. However, you need to do things correctly. Again, see the "Boilerplate" section above.

Oh, and PEP 8 recommends against underscores in module names unless it improves readability, but I think mylxml is readable enough. However, simply adding my in front of a module is not such a good idea, because you just lost an opportunity to explain what makes your lxml different. Since it seems specific to the file specification you use, why not use that in the name?


Keep track on active slide I am using class "active" to keep track on which child to fade in and fade out in my gallery. Are there any other, better options for doing this?

<!DOCTYPE html>

<meta charset="utf-8" />
<title>Untitled Document</title>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">

1

2

3

4

Maybe using jQuery's $.data() to store a bool variable or something like that.

Also you could store a global variable with the active position (e.g. "3" for the 3rd item in the gallery), and then change it, using it along with the :eq() selector.

Either way, I think that the .active class approach is very useful because it gives you the chance to alter the CSS properties for that specific item on your gallery.


Remove indicies from vector I am trying to optimise my remove-erase functions by using some attributes of C++ containers. This function should delete all indices defined in std::set form given vector. I am pretty sure that algorithm is functionality-wise working, but every time I see STL implementation or any code using STL I see a lot of checks around which I cannot come up when I am writing my own code.
I decided to practise by trying so I hope for some tips or tricks on how to improve my code-writing skills.
The algorithm is trying to prevent unnecessary copies. So it is copying just the elements that are supposed to survive. It goes by two iterators. One for a destination where the next element should be copied and second the next element that should "survive". When it arrives at the end of the vector minus the number of deleted elements it stops and leaves the rest for erase() function.
//=================================================================================
// Deletes all indexes defined in set from vector. Leverages the advantage of set being
// sorted from the smallest to the highest number and continuity of vector storage.
template <class T, class IdxT>
typename std::vector<T>::iterator remove_indices(std::vector<T>& vector, const std::set<IdxT>& set)
{
    if (set.empty())
    {
        return vector.end();
    }

    auto       nextDelete       = set.begin();
    const auto destinationIndex = *nextDelete;
    auto       destination      = vector.begin();
    auto       source           = vector.begin();
    std::advance(destination, destinationIndex);
    std::advance(source, destinationIndex + 1);
    ++nextDelete;
    auto sourceIndex = destinationIndex + 1;
    while (destination != vector.end() - set.size())
    {
        while (nextDelete != set.end())
        {
            if (sourceIndex == *nextDelete)
            {
                ++source;
                ++nextDelete;
                ++sourceIndex;
            }
        }
        *destination = std::move(*source);
        ++destination;
        ++source;
        ++sourceIndex;
    }
    return destination;
}

There are a few things I would improve:

Clearly separate the three distinct phases:

Skip until the first index in the set
Copy/delete until we reached the last index
Copy the remainder until the end of the container is reached.

Have an outer loop that loops over the entries of set.
Use *foo++ syntax.
Use auto return type deduction.
Name things after their purpose, not what type they are. So instead of set, name it indices.

With this, the code would look like:
template <class T, class IdxT>
auto remove_indices(std::vector<T>& container, const std::set<IdxT>& indices)
{
    // Exit early if there is nothing to do.
    if (indices.empty())
        return container.end();

    auto destination = container.begin();
    auto source = container.begin();
    auto idx_it = indices.begin();

    // Skip until we reach the first index.
    std::advance(destination, *idx_it);
    std::advance(source, *idx_it + 1);

    // Delete all the elements at the given indices.
    while(++idx_it != indices.end()) {
        auto nextDelete = container.begin() + *idx_it;
        while(source < nextDelete)
            *destination++ = std::move(*source++);
        source++;
    }

    // Copy the remainder until the end of the container.
    while (source != container.end())
        *destination++ = std::move(*source++);

    return destination;
}

Also consider adding a test suite. And you might want to generalize this further to remove elements from other random access container types (for example, std::deque), and have it take the indices in other container types as well, as a std::set is quite an inefficient way to store a few integers.


clean way to make array a 2d array Looking for a cleaner (possibly one liner) way to write the following

my $spec_2d = ( );
foreach ( @spec ) {
  $spec_2d[$_][0] = $spec[$_];
}
@spec = @spec_2d;

Basically I'm making an array an array of arrays

@spec_2d[@spec] = map [ $spec[$_] ], @spec;

but judging from the subject I think you want this,

my @spec_2d = map [ $_ ], @spec;


Design dilemma: extensibility vs simplicity Here is my problem abstracted to Bird classes. I know that number of Birds will increase on the future and new behaviors might be needed. With 10 Birds first design might not look so simple and lead to lots of duplicated code. On the other hand, second design can be perceived as "class explosion". Which of these two designs would be considered best-practice?

Classic:

#include <iostream>

class Bird
{
  public:
    virtual void Fly() const = 0;
    virtual void Speak() const = 0;
};

class Eagle : public Bird
{
  public:
    virtual void Fly() const
    {
      std::cout << "Eagle shall fly now!" << std::endl;
    }
    virtual void Speak() const
    {
      std::cout << "Eagle speaking!" << std::endl;
    }
};

class Penguin : public Bird
{
  public:
    virtual void Fly() const
    {
      std::cout << "Penguin shall fly now!" << std::endl;
    }
    virtual void Speak() const
    {
      std::cout << "Penquin speaking!" << std::endl;
    }
};

int main()
{
  std::cout << "..." << std::endl;
  Bird* bird = NULL;

  bird = new Eagle();
  bird->Fly();
  bird->Speak();
  delete bird; bird = NULL;

  bird = new Penguin();
  bird->Fly();
  bird->Speak();
  delete bird; bird = NULL;

  return 0;
}

"Better?

#include <iostream>
#include <cassert>

class FlyStyle
{
  public:
    virtual void Fly() const = 0;
};

class FlyHigh : public FlyStyle
{
  virtual void Fly() const
  {
    std::cout << "Fly high!" << std::endl;
  }
};

class NoFly : public FlyStyle
{
  virtual void Fly() const
  {
    std::cout << "No fly!" << std::endl;
  }
};

class SpeakStyle
{
  public:
    virtual void Speak() const = 0;
};

class SpeakLoud : public SpeakStyle
{
  virtual void Speak() const
  {
    std::cout << "Speak LAUD!!!!" << std::endl;
  }
};

class NoSpeak : public SpeakStyle
{
  virtual void Speak() const
  {
    std::cout << "No speaking!" << std::endl;
  }
};

class SuperBird
{
  public:
    SuperBird(FlyStyle* fly, SpeakStyle* speak)
      : flystyle(fly),
      speakstyle(speak)
  {
    assert(NULL != flystyle);
    assert(NULL != speakstyle);
  }

    ~SuperBird() { delete flystyle; delete speakstyle;}

    virtual void Fly() const
    {
      flystyle->Fly();
    }
    virtual void Speak() const
    {
      speakstyle->Speak();
    }
  protected:
    FlyStyle* flystyle;
    SpeakStyle* speakstyle;
};

class SuperBirdFactory
{
  public:
    static SuperBird* createEagle()
    {
      return new SuperBird(new FlyHigh(), new SpeakLoud());
    }
    static SuperBird* createPenguin()
    {
      return new SuperBird(new NoFly(), new NoSpeak());
    }
};

int main()
{
  SuperBird* bird = NULL;

  bird = SuperBirdFactory::createEagle();
  bird->Fly();
  bird->Speak();
  delete bird; bird = NULL;

  bird = SuperBirdFactory::createPenguin();
  bird->Fly();
  bird->Speak();
  delete bird; bird = NULL;

  return 0;
}

In my opinion, a better approach (along similar lines) is to avoid abstract base classes and instead use generic "policy" classes. You informally define an interface for each behaviour, and mix them into the class as template parameters. This avoids the need for dynamic memory allocation, and removes the overhead of virtual function calls; everything is resolved at compile time. Your example could be something like this:

#include <iostream>

// Flying styles must have interfaces compatible with this
// struct FlyStyle
// {
//    void Fly() const;
// };

struct FlyHigh
{
  void Fly() const
  {
    std::cout << "Fly high!" << std::endl;
  }
};

struct NoFly
{
  void Fly() const
  {
    std::cout << "No fly!" << std::endl;
  }
};

// Speaking styles must have interfaces compatible with this
// struct SpeakStyle
// {
//     void Speak() const;
// };

struct SpeakLoud
{
  void Speak() const
  {
    std::cout << "Speak LAUD!!!!" << std::endl;
  }
};

struct NoSpeak
{
  void Speak() const
  {
    std::cout << "No speaking!" << std::endl;
  }
};

template <class FlyStyle, class SpeakStyle>
class SuperBird
{
  public:
    void Fly() const
    {
      flystyle.Fly();
    }
    void Speak() const
    {
      speakstyle.Speak();
    }
  private:
    FlyStyle flystyle;
    SpeakStyle speakstyle;
};

typedef SuperBird<FlyHigh, SpeakLoud> Eagle;
typedef SuperBird<NoFly, NoSpeak> Penguin;

int main()
{
  Eagle eagle;
  eagle.Fly();
  eagle.Speak();

  Penguin penguin;
  penguin.Fly();
  penguin.Speak();
}


Display data based on read configuration file from ssh sessions I started playing with Python last week. This is a section of code from an application which reads a configuration file and launches ssh sessions and continuously displays data back to the local user.

The whole project is on Github.

def open_connections(sessions):
    """ open_connections(sessions)
    Opens ssh connections and forks them to the background. Passwords are sent
    from the user via STDIN to each process. If correctly identified, the
    process is successfully created and added to the Session object.
    """

    for server in sessions:
        usr = raw_input("Enter username for " + server.address + ": ")
        pwd = getpass.unix_getpass("Enter passphrase for " + server.address + ": ")
        con = paramiko.SSHClient()
        con.set_missing_host_key_policy(paramiko.AutoAddPolicy())
        con.connect(server.address,
                    username=usr,
                    password=pwd)
        print "[OK]"
        server.connection = con

def run(sessions):
    """ run(sessions)
    Takes the file descriptors from the open sessions and runs the commands
    at the specified intervals. Display the STDOUT to the current tty.
    """

    while True:
        time.sleep(TIMEOUT)
        os.system("clear")
        for session in sessions:
            print session.command + " @ " + session.address
            print "------------------------------------------------------------"
            stdin, stdout, stderr = session.connection.exec_command(session.command)
            print format_stdout(stdout.readlines())

A couple of my concerns:

Am I doing any big Python no-nos?
Am I following proper comment/code conventions?
Is this the best way to display continuous information to user (screen refreshes)?

This is my first contact with Python so I'm definitely doing something I shouldn't be.  The rest of the code for this project is really only 1 file, and what I posted above is the bulk of it. If anyone went to the Github page and wanted to comment on something else, I'd be happy to respond.

usr = raw_input("Enter username for " + server.address + ": ")
    pwd = getpass.unix_getpass("Enter passphrase for " + server.address + ": ")
    con = paramiko.SSHClient()

I'd avoid using abbreviations. Call them username and password and connection. You don't gain anything using abbreviations and it reduces readability.

    for session in sessions:

for server in sessions:

Are they servers or sessions?

I'd also move the contents of the loop into methods on your session objects.


Python startswith and then look a word in a conditional block I have below working code which works fine, but just looking way around if there is an another elegant way of doing this, because here i'm using else condition to print the last line or dataBlock to be printed.

Note: mydudalt1 and mydudalt2 are the host names

My data File:

$ cat mydata_test.txt
-- mydudalt1 --
            192.168.2.40;       // udalt-sanjose
            192.168.2.56;       // udalt-sj1
            192.168.98.71;        // udalt-japan
            192.168.2.146;      //udalt-sj-fwd1

-- mydudalt2 --
      199.43.4.70;  // udalt-chelms
      192.168.2.56; // udalt-sj1
      192.168.98.71; // udalt-japan

My Working code:

#!/grid/common/pkgs/python/v3.6.1/bin/python3
dataBlock = ''
with open('mydata_test.txt', 'r') as frb:
    for line in frb:
        line = line.strip("\n")

        if line.startswith('--'):
            if "japan" in dataBlock:
                print(dataBlock)
            dataBlock = ''
            dataBlock = dataBlock +  line

        elif "japan" in line:
            dataBlock = dataBlock +  line
    else:
        print(dataBlock +  line)

Resulted Output:

-- mydudalt1 --            192.168.98.71;        // udalt-japan
-- mydudalt2 --      192.168.98.71; // udalt-japan

Unless there is an indentation issue,

        dataBlock = ''
        dataBlock = dataBlock +  line

can be written:

        dataBlock = line


Create multiple kml files in Python I am new to Python programing language. I have created the codes, based on this solution.

I prepared two simple programs:

import simplekml

List2 = [ [ 'Placemark','old file', 51.500152, -0.126236 ] ] # description, lat, lon
List3 = [ [ 'New placemark','new file', 51.600152, -0.136236 ] ]

kml = simplekml.Kml()
for row in List2:
    kml.newpoint(name=row[0], description=row[1],
    coords=[(row[3], row[2])])  # lon, lat, optional height
for row in List3:
    kml.newpoint(name=row[0], description=row[1],
    coords=[(row[3], row[2])])

kml.save("test2.kml")

and from another file here:

import simplekml
kml = simplekml.Kml()
kml.newpoint(name="Kirstenbosch", description="Description", coords=[(18.432314,-33.988862)])  # lon, lat,optional height
kml.newpoint(name="Kirstenbosch2", description="<b> Botanic garden </b> in South Africa", coords=
 [(18.532314,-33.788862)])
kml.save("output/botanicalgarden2.kml")

I know that this code (simplekml tool) can be used for the bulk .kml data, as it was mentioned here.

Could you tell me how to write this code not repeatedly, as I did? I would like to have a multitude of place mark locations within one list/new point, when possible.

In your first code you already have everything you need, you just don't use it. List2 and List3 are each lists of points to create. But they only contain one point, so your for loop does nothing exciting, it only runs once.

You can simply add more points to any of the lists, here I take the first as an example:

import simplekml

points = [['Placemark', 'old file', 51.500152, -0.126236],
          ['New placemark', 'new file', 51.600152, -0.136236]] # description, lat, lon

kml = simplekml.Kml()
for point in points:
    kml.newpoint(name=point[0], description=point[1],
                 coords=[(point[3], point[2])])  # lon, lat, optional height

kml.save("test2.kml")

You could make this a bit more readable by using a collections.namedtuple for the markers. This way your comments become superfluous:

from collections import namedtuple
import simplekml

Marker = namedtuple("Marker", ["name", "description", "lat", "lon"])

points = [Marker('Placemark', 'old file', 51.500152, -0.126236),
          Marker('New placemark', 'new file', 51.600152, -0.136236)]

kml = simplekml.Kml()
for point in points:
    kml.newpoint(name=point.name, description=point.description,
                 coords=[(point.lon, point.lat)])

kml.save("test2.kml")

Note that I followed Python's official style-guide, PEP8, when naming my variables. Variables and functions should be lower_case and only classes (such as the namedtuple class I create) should be in PascalCase. PEP8 also recommends putting a space after commas and not to use unnecessary whitespace.


POSIX shell function to "dump" all given arguments My today's idea was to create a POSIX shell function to dump all given arguments, typical use would be to call it from a fucntion, where you already know some arguments are not well set (empty; not integer) and this way you can inspect all given arguments in one go. It reminds me of a PHP var_dump function. :) Would anyone be able to find weak spots in there, all input is welcome.

#!/bin/sh

dump_arguments ()
# string function - prints arguments (position and content)
# indicates empty arguments and integer numbers
{
    printf '%b' "dump_arguments()\\n----------------\\n$# arguments are being inspected"
    [ $# -gt 0 ] && { printf ':\n'; i=1; } || printf '.\n'
    while [ $# -gt 0 ]; do
        printf '%s' "[$i]: '$1'"
        [ -z "$1" ] && printf ' (empty)'
        [ "$1" -eq "$1" ] 2> /dev/null && printf ' (integer)'
        printf '\n'
        shift 1
        i=$((i+1))
    done
} >&2

# inside some function you would call it like this
dump_arguments "$@"

# but to only try it out, you can call it directly
dump_arguments '1' '' 5

printf '%b' "dump_arguments()\\n----------------\\n$# arguments are being inspected"

We could make that easier to read by making each line a separate argument, and ending the line properly (rather than requiring a separate command):
punct=.
${1+false} true || punct=:
printf '%s\n'                                       \
       'dump_arguments()'                           \
       '----------------'                           \
       "$# arguments are being inspected$punct"

It seems odd to use %s here:

    printf '%s' "[$i]: '$1'"

Why not use the format string more clearly?  And this allows us to modify to use %q if we have a suitable printf:
    printf "[%d]: '%s'" "$i" "$1"

Perhaps even combine the type into a single print:
    type=
    [ -z "$1" ] && type=' (empty)'
    [ "$1" -eq "$1" ] 2>/dev/null && type=' (integer)'
    printf "[%d]: '%s'%s\n" "$i" "$1" "$type"

Is it intentional that strings such as ' 5' (with leading and/or trailing spaces) are counted as integers?

    shift 1

Normally written simply as
    shift

That said, I think it's more natural to iterate over arguments using a for loop instead.

} >&2

That's surprising, as the output isn't an error, but expected when calling the function.  It should be up to the caller to choose where stream 1 goes.

Modified code
#!/bin/sh

dump_arguments()
# string function - prints arguments (position and content)
# indicates empty arguments and integer numbers
{
    punct=${1+:}
    printf '%s\n'                                               \
           'dump_arguments()'                                   \
           '----------------'                                   \
           "$# arguments are being inspected${punct:-.}"
    i=1
    for v
    do
        if [ -z "$v" ]
        then type=' (empty)'
        elif [ "$v" -eq "$v" ] 2>/dev/null
        then type=' (integer)'
        else type=
        fi
        printf "[%d]: '%s'%s\n" \
               "$i"   "$v" "$type"
        i=$((i+1))
    done
}


Processing concurrent AJAX requests I am trying to process concurrent AJAX requests with vanilla JavaScript. Below is the sample code I've been working with

function getInfoViaConcurrency(ids) {
  let infoPromiseArr = ids.map(id=> { infoPromiseArr.push(getInfoViaAjax(id)); });

  return Promise.all(infoPromiseArr).then(values => { return values; });
}

function getInfoViaAjax(id) {
  return new Promise((resolve, reject) => {
    let  xhr = new XMLHttpRequest();
    xhr.open('GET', `API URL`);
    xhr.onload = function() {
      if (xhr.status === 200) {
        resolve(JSON.parse(xhr.responseText));
      }
      else {
          reject(xhr.status);
      }
    };
    xhr.send();
  })
}

Does using promise.all suffice to promote concurrency? can someone suggest any improvements?

Does using promise.all suffice to promote concurrency?

It's not really Promise.all that does it. You're starting your requests without waiting for the previous ones to complete, so yes, they run concurrently. All Promise.all does is let you know when all of the promises you give it have resolved (or when the first one rejects).

  can someone suggest any improvements?

Some thoughts:

In getInfoViaConcurrency:

Your use of map is incorrect, kind of a mash-up of your original forEach (from your now-deleted post on SO) and map
You can pass getInfoViaAjax directly to map
The then handler on the Promise.all serves no purpose and can be removed

In getInfoViaAjax, you could use fetch on modern browsers rather than XMLHttpRequest

So getInfoViaConcurrency could be:

function getInfoViaConcurrency(ids) {
  return Promise.all(ids.map(getInfoViaAjax));
}

and if you want to use fetch, getInfoViaAjax could be:

function getInfoViaAjax(id) {
  return fetch({/*...appropriate options and URL...*/}).then(response => {
      if (!response.ok) {
          throw new Error(/*...whatever you want to say here...*/);
      }
      return response.text();
  });
}


Hour averaging program I am looking for tips on improving my short program.  I am using system("PAUSE") because this was for an assignment.

Code:

#include <iostream>
#include <string>
#include <iomanip>
using namespace std;

void Display(string* , int* , double*, int);
void Percentage(int*, double*, int);
void Intro(string*, int*, int&);
void Highest(int&, string&, double*, string*, int);

int main()
{
    int students;
    string names[10];
    int hours[10];
    double percents[10];
    int highest;
    string most;

    Intro(names, hours, students);

    Percentage(hours, percents, students);
    Display(names, hours, percents, students);

    Highest(highest, most, percents, names, students);

    system("PAUSE");
}

void Intro(string* names, int* hours, int& students)
{
    int team;
    cout << "A team is made up of atleast 2 students. How many students are on the team?: ";
    cin >> team;

    cout << "Enter student's first name and the hours worked on the final project: " << endl;

    for (int i = 0; i < team; i++)
    {
        cout << i + 1 << ": ";
        cin >> names[i] >> hours[i];
    }

    students = team;
}

void Display(string* names, int* hours, double* percent, int students)
{
    cout << setw(20) << "Students";
    cout << setw(20) << "Hours Worked";
    cout << setw(20) << "% of Total Hours";
    cout << endl;
    cout << "------------------------------------------------------------" << endl;
    for (int i = 0; i < students; i++)
    {
        cout << setw(20) << names[i];
        cout << setw(9) << hours[i];
        cout << setw(16) << percent[i];
        cout << endl;
    }
}

void Percentage(int* hours, double* percent, int students)
{
    int total(0);

    for (int i = 0; i < students; i++)
    {
        total += hours[i];
    }

    for (int i = 0; i < students; i++)
    {
        percent[i] = double(hours[i]) / total * 100;
    }
}

void Highest(int& highest, string& most, double* percent, string* names, int students)
{
    highest = percent[0];

    for (int i = 0; i < students; i++)
    {
        if (highest < percent[i])
        {
            highest = percent[i];
            most = names[i];
        }
    }

    cout << most << " worked the most hours." << endl;
}

I assume you'll have no more than 10 names/hours/percents, but you could use an std::vector in place of these arrays.  This will allow you any number of inputs without fear of exceeding the allotted 10.  If you don't need this for your assignment, then you can stick with what you have.
If you're allowed to use more of the STL, I'd recommend std::accumulate for summing up the values in a container.

For instance, your first loop in Percentage():

for (int i = 0; i < students; i++)
{
    total += hours[i];
}

can be done with this function as such (with the array):

// the 10 corresponds to the array size
// the 0 is the starting value of the accumulator

int total = std::accumulate(hours, hours+10, 0);

If you choose to use std::vector (or other STL container):

// functions cbegin() and cend() return const iterators

int total = std::accumulate(hours.cbegin(), hours.cend(), 0);

I'd slightly tweak the percentage calculation for clarity:

percent[i] = (double(hours[i]) / total) * 100;

Prefer to cast the C++ way:

// C way
double(hours[i])

// C++ way
static_cast<double>(hours[i])

It seems a little weird having Highest() display something.  You could instead return most, thereby making the function of type std::string.  That way, you can display this name from main() or wherever else.

You don't need the first two arguments.  They are passed in from main(), but also not modified by any previous functions.  I'd remove those two and just use local variables.

highest should be of type double since it's being assigned to a percent element.  Otherwise, you may receive a "possible loss of data" warning.  This is especially important for when the two values are being compared in the loop.

You could then have this:

// get the name
std::string most = Highest(percents, names, students);

// display it
std::cout << most << " worked the most hours." << std::endl;

std::string Highest(double* percent, string* names, int students)
{
    double highest = percent[0];
    std::string most;

    for (int i = 0; i < students; i++)
    {
        if (highest < percent[i])
        {
            highest = percent[i];
            most = names[i];
        }
    }

    return most;
}


Representing a recursive data structure as HTML DIVs I am using the following code which recursively executes function findWpisInZone(), code works fine, I just wondering if there is any better way to do it in JS,

DomBuilder.prototype = {
            domTree: '',
            findWpis: function () {
                var dataState = dojo.global.xxx.state.data,
                    scope = this;
                for (var zone in dataState) {
                    for (var wpi in dataState[zone]) {
                        this.domTree += '
'.format(zone);
                        findWpisInZone(dataState[zone][wpi]);
                        this.domTree += '
';
                    }
                }
                function findWpisInZone(wpi, type) {
                    scope.domTree += '
'.format(wpi.wpi.webpart_cid);
                    for (var zone in wpi.zones) {
                        scope.domTree += '
'.format(zone)
                        for (var wpiInner in wpi.zones[zone]) {
                            // recursion search
                            findWpisInZone(wpi.zones[zone][wpiInner], type);
                        }
                        scope.domTree += '
';
                    }
                    scope.domTree += '
';
                }
            },

I think what you've got here is looking pretty fine. You could move the comment explaining that it's recursive to the function header if you want it to be more explicit. The recursive function isn't doing anything strange, though.

Usually, with recursion, one can unroll the recursion to be iteration instead.

I wouldn't recommend that in this case. Right now, you have about 10-12 lines, depending on how you count. It's just two forloops. The whole function can be summarized as "make a div that contains, for each wpi zone, another div containing it's sub wpi's, similar to the div we make now."

If you were to unroll this recursion then it would get messy.

You did well.

Possible improvements are assigning the strings to separate constants, and perhaps storing wpi.zones[zone] in a local store to reduce a double double array access for each for-loop iteration to a double single array access (the for loop accesses [zone][0] and [zone][wpiInner], by storing [zone] locally, you reduce the accesses to [0] and [wpiInner] instead).


Python - Random Mandala I did a project in Python where I combined user-input, random, and the turtle module to make a sort of "Mandala Generator", or a program that can generate a simple design either randomly or by the specifications of users. Is there any way I can shorten my code without drastically changing it?

#custom mandala project Python
import turtle
import random
import time

#is the game in progress?
drawing = False

#define turtle
pen = turtle.Turtle()
pen.setheading(90)
pen.pensize(3)
pen.speed(12)

#colors empty list
colors = []

#define variables
LINE1, LINE2 = "~" * 36, "~" * 24

#define check input function
def get_input(response, values):
    #make sure the input is in the choices
    while True:
        value = input(response).strip().lower()
        if value in values:
            return value

#define main function
def main():
    print(LINE1 + "\nCustom Mandala Generator\n" + LINE1)
    name = input("What is your name? ")
    print("In this custom mandala generator, you get to choose the size and colors of your mandala, or have the computer generate a random one,", name, ".\n" + LINE2)
    time.sleep(1)
    ready = input("Are you ready or not? ")

    if ready[0].lower() == "y":
        answer = get_input("Would you like to use our random generator function or our custom function? {random/custom} ", {"random", "custom"})

        #universal variables
        size = random.uniform(1.5, 2.5)
        fd = 75 * size

        if answer == "random":
            colors.extend(("darkred", "red", "yellow", "darkgreen", "green", "lightgreen", "darkblue", "blue", "purple"))
            #rt = right turn
            rt = random.uniform(100, 300)

        elif answer == "custom":
            print(LINE2 + "\nYou can choose any colors from this list for your Mandala: \nDarkred, red, yellow, darkgreen, green, lightgreen, darkblue, blue, purple")
            color1 = get_input("What is the first color? ", {"darkred", "red", "yellow", "darkgreen", "green", "lightgreen", "darkblue", "blue", "purple"})
            color2 = get_input("What is the second color? ", {"darkred", "red", "yellow", "darkgreen", "green", "lightgreen", "darkblue", "blue", "purple"})
            colors.extend((color1, color2))
            rt = int(input("What angle would you like your Mandala to turn at? "))

        #pre-draw sequence
        print(LINE2 + "\nInitializing...")
        print("Determining features... \nColor... \nSize...")
        time.sleep(3)
        print(LINE1 + "\nYour final result should be drawing as of this point!\nThanks for using the generator,", name, "!")

        for counter in range(50):
            selection = random.choice(colors)
            pen.color(selection)
            pen.forward(fd)
            pen.right(rt)

    else:
        print("Come and use the custom generator later!")

main()

I don't know why you would want to shorten your code. It is already just 73 lines long (including the final blank line). Apart from that, "short" does not automatically mean "good". Maybe concise would be a better fit here, which I would interpret as "the code does what it should do without clutter". IMHO your code is very concise and readable at the moment and that should not be sacrificed to gain a few lines/bytes. We're not at Code Golf here.

Nevertheless, there are a few things you can improve easily.

Documentation

Python's official Style Guide has recommendations on how to comment code and functions. Function docstrings should always be defined inside the function body and be surrounded by triple quotes """...""". Applying this to your code it would look something like

def get_input(response, values):
    """make sure the input is in the choices"""
    # your code here

This will also allow Python's built-in help(...) function and most IDEs to pick up the documentation.

Avoid globals

You should always strive to avoid global variables whenever possible. E.g. in your code colors is not used outside of main() and should therefore be defined there. drawing does not seem to be used anywhere, so you might be able to drop it completely. LINE1 and LINE2 are two valid uses for globals. My only recommendation here would be to put them on two separate lines. Oh, and strictly speaking your comment on them is "wrong" since they are not used as variables but as constants, which is also supported by the ALL_CAPS_NAME (the Style Guide also says something on that).

Variable names

There is this piece of code which I find kind of amusing:

#rt = right turn
rt = random.uniform(100, 300)

Why not just call the variable right_turn or right_turn_angle right away then? The same reasoning would apply to fd, but you didn't care to write the comment there and left it as an exercise to the person looking at your code to find out that it actually stands for forward. You could also rename size to mandala_scale or so and then drop that #universal variables altogether since it does not really help to understand the code better. At least at the moment you could even get rid of size completely since its only use is to be multiplied by 75 on the next line and it's never used after that.

Other aspects

There is a lot of repetition when it comes to the list of available colors. Including the print, you define them four times. If you ever decide to add, change or remove a color from that list, there would be four places that change would need to be applied to. I would recommend to define them in one place, and reuse the list wherever it is needed.

# ...

colors = []
all_colors = ("darkred", "red", "yellow", "darkgreen", "green", "lightgreen", "darkblue", "blue", "purple")
if answer == "random":
    colors.extend(all_colors)
    right_turn = random.uniform(100, 300)

elif answer == "custom":
    all_colors_string = ", ".join(all_colors)[:-2] # -2 to trim of the trailing space and ,
    print(LINE2 + "\nYou can choose any colors from this list for your Mandala: \n" + all_colors_string)
    color1 = get_input("What is the first color? ", all_colors)
    color2 = get_input("What is the second color? ", all_colors)

# ...

In the transformation the set notation ({...}) got lost, but I think your application will not really suffer from the (minuscule) performance impact because a tuple is used instead of a set.

Another handy feature I would like to point out to you is string formatting. So what's that? Let's look at an example from your code:

print("[...] or have the computer generate a random one,", name, ".\n" + LINE2)

Here you are using Python's implicit behavior to print the elements of a tuple separated by whitespace1. That leads to the following output (minus the annotation of course):

[...] or have the computer generate a random one, Alex .
~~~~~~~~~~~~~~~~~~~~~~~~
                                       does look off ~^

Note the unwanted whitespace between the name and the dot?
But we can do better! Python has very powerful string formatting capabilities (doc), and since you have tagged your question as Python 3, you can likely use the new f-string syntax (helpful blog post comparing all the different ways, specification (PEP 498)).
Rewriting the code from above using an f-string would lead to:

print(f"[...] or have the computer generate a random one, {name}.\n" + LINE2)

So f"... {name}" basically generates a string containing the value of name right at the spot we the curly braces and the name of the variable where.2 For the sake of completeness that generates the following output:

[...] or have the computer generate a random one, Alex.
~~~~~~~~~~~~~~~~~~~~~~~~

As a final note, you could/should also add

if __name__ == "__main__":
    main()

(explanation) to highlight what part of the code is actually run if this file is used as Python script.

Bonus: by removing unnecessary globals and comments your script is actually even shorter more concise now, although that was not the main goal of what I was trying to tell you.

1 Depending on your Python experience you might wonder where there is a tuple in that line or you will nod silently. In the first case I would recommend reading this SO post on how Python deals with functions that have a variable number of arguments (often varargs for short), and then to have a look at how the print function is defined in the documentation.
2 In fact, f-strings are way more powerful than this, but you should refer to the resources linked earlier if you want to know more.