Bloch's Builder Pattern / Updater I'm creating many builders where I use inheritance.

My resources have many properties and are stored in a Repository.
In rare case I need to update them.
When I update, I need to keep the same objectId (I track update on the repository).

I use the same Builder/Updater as it's easier for my users (there are more than 10 concrete builders).

The code with only one builder (and properties removed):

     public interface IBuilder<T, P>
        {
            P Build();
            P Update();
        }

    public abstract class BuilderBase<T, P>: IBuilder<T, P>
        where T : BuilderBase<T, P>
        where P : class, new()
        {
            protected P Result;
            protected P Source;
            protected T This;

            protected BuilderBase()
            {
                Result = new P();
                This = (T) this;
            }

            protected BuilderBase(P source):this()
            {
                Source = source;
            }

            public P Build()
            {
                var result = Result;
                Result = null;
                return result;
            }

            public abstract P Update();
    }

   public abstract class ResourceBuilder<T, P> : BuilderBase<T, P>
        where T : ResourceBuilder<T, P>
        where P : ResourceBase, new()
        {

            protected ResourceBuilder():base(){}

            protected ResourceBuilder(P resource):base(resource)
            {
                Result.Name = Source.Name;
            }

            public new P Build()
            {
                var result = Result;
                return base.Build();
            }

            public override P Update()
            {
                if (Source != null)
                    Result.ResourceId = Source.ResourceId;
                return Build();
            }

            public T Name(string name)
            {
                Result.Name = name;
                return This;
            }

        }

    public abstract class ColorResourceBuilder<T, P> : ResourceBuilder<T, P>
           where T : ColorResourceBuilder<T, P>
           where P : ColorResource, new()
    {

        public T Color(Color color)
        {
            Result.Color = color;
            return This;
        }

        protected ColorResourceBuilder(P colorResource) : base(colorResource)
        {
            Result.Color = Source.Color;
        }

        protected ColorResourceBuilder() : base()
        {
        }
    }

       public class ColorResourceBuilder : ColorResourceBuilder<ColorResourceBuilder, ColorResource> {
        public ColorResourceBuilder(ColorResource colorResource) : base(colorResource)
        {
        }

        public ColorResourceBuilder()
        {
        }
    }

The main problem with this approach is for the Update():

var colorResource2 = new ColorResourceBuilder(colorResource1).Name("Blue").Update();

colorResource2 != colorResource1  => two different object have the same Id...

hypothetical improvement:

var colorResource2 = new ColorResourceBuilder(ref colorResource1).Name("Blue").Update();
var colorResource2 = new ColorResourceBuilder(ref colorResource1).Name("Blue").Build();

It could be OK for the Update(). But weird with Build() as ref suggest update.

Another hypothetical improvement:

var colorResource2 = new ColorResourceBuilder().Name("Blue").Update(ref colorResource1);
var colorResource2 = new ColorResourceBuilder(colorResource1).Name("Blue").Build();

Will incur another usage inconsistency problem.

I'm looking for any advice to make it more clean and natural. Or any other pattern which could do the job.

Comment on the builder pattern

I don't think that Joshua Bloch discovered any new version of the builder pattern. I suggest to follow the standard way and purposes from the GoF.

The purposes of the builder pattern are:

avoid having a lot of constructor parameters
provide a default and preserve consistency to be able to ALWAYS build an object
make the code fluently readable

I think here the pattern is not appropriate and not implemented in the way it should be. The Update()-method for example has nothing to do with any builder pattern.

A guess on the intention

As you mentioned a "repository" you are dealing with some kind of persistence context. In your implementation the build()-method will create a new object in the repository and the update()-method wil update an existing object in the repository so far I understand.

Proposal, creation and result

The design I would follow is a natural approach.

If you want to create a new resource then you create a proposal object that contains data that may be considered during a creation process. The proposal object itself may have been constructed by a builder pattern. That depends on the purposes mentioned in the first place.

After that the proposal object is passed into the creation process. It will be validated and checked against internal system consistency. The creator object is well defined as the responsible for the creation of the wished object (SRP).

If no validation errors occured and it is not violating the systems consistency the object ist created AND persisted. The object may be internally (in the responsible object) constructed through the builder pattern. But that has nothing to do with persistence.

Proposal, update and result

The same with the update. The only difference is that you have an update proposal, an existing object to update and a responsible object that will to the update process.

Now it's getting interesting. The result of the update can be a versioned new object xor a cached old object. This depends on if you want to work with immutability or not. As you have an "id" I suggest to have the objects cached and mutable.

Nevertheless you have to override equals and hashcode in both scenarios. But be careful: two objects of any version are equal if their id is equal.

Conclusion

Do not make the builder pattern return mutable objects, always return new objects. The "builder" pattern is meant to "build" something not to update something
Only build transient, never involve persistence in the builder pattern
Have a proper design for CRUD-Operations for your persistent objects (business objects??) and well defined responsibilities
Distinguish between proposal, creator/updater/deleter and result

I am sorry, this is more a design issue of your problem domain than a pattern issue.


Print Student Names from CSV Description

Print all student names from a CSV file.

CSV Format:

CSV file does not contain string delimited by double quotes.
None of the cells has "," character or "\"" character.
There are exactly two columns.
No empty cells.

Code

csvread.pl

#!/usr/bin/perl

use strict;
use warnings;

my $filename = "students.csv";
open (STUDENTS,$filename) ||  die "Cannot Open '$filename'";

# get column names
my $line = <STUDENTS>;
chomp($line);
my @column_names = split(",",$line);

print "Printing Student ${column_names[1]}s...\n";

# get all names
while($line = <STUDENTS>){
    chomp($line);
    (my $id,my $name) = split(",",$line);
    print "$name\n";
}

#--------------------------------------------------------
# documentation
#

=head1 NAME

Print Student Names

=head1 SYNOPSIS

    chmod a+x ./csvread.pl
    ./csvread.pl

=cut

students.csv

ID,Name
1,Ashani
2,Sandeepani
3,Nimesha
4,Kaveesha
5,Abimani
6,Himadu
7,Vishmi
8,Shela
9,Vishvani
10,Claudia
11,Dinuwanthi
12,Anthonella
13,Sachini
14,Hiruni
15,Nimesha
16,Kavindi
17,Roshini
18,Dilini
19,Devinka
20,Noela
21,Nithmi
22,Azekah
23,Ishara
24,Andriya
25,Rishini
26,Shashini
27,Oshadhi
28,Sandali
29,Jehani
30,Thamasha
31,Shalini
32,Milka
33,Gihani
34,Sakuni
35,Piumi
36,Stefania
37,Kinkini
38,Shehana
39,Nimanshi
40,Emalsha
41,Shohani
42,Madusha
43,Nisaja
44,Kiyara
45,Dinithi
46,Sadunika
47,Sewmini
48,Rachel

Review

Conventions/Generic

It looks quite good.  Perldoc, use strict, use warnings are all good practices.  The formatting is good, and the code is readable.

I don't recommend hard-coding the name of the data file in your program.  That turns it from a general-purpose program into throwaway code.  If you use the default filehandle <>, then Perl will do the "right thing": if a filename is specified as a command-line parameter, it will read from that file; if not, it will read from STDIN.  As a bonus, it saves you the trouble of having to call open().  (You neglected to explicitly close(STUDENTS), by the way.)

The CSV-parsing code is repeated for the header row and for the body.  It should be made into a subroutine.

(my $id, my $name) could be written more idiomatically as my ($id, $name).

I find it annoying to use string interpolation like print "$something\n" just because you need a newline.  I suggest taking advantage of the newish say feature.

The facts that you want the second column, and that the columns are the $id and $name, are both hard-coded.  Perhaps you should add an assertion.

Suggested implementation

use strict;
use warnings;
use feature qw(say);

sub columns {
    my $line = shift;
    return unless defined $line;
    chomp($line);
    split ',', $line;
}

my @column_names = columns(scalar <>);

die unless $column_names[1] eq 'Name';
say "Printing Student ${column_names[1]}s...";
while (my ($id, $name) = columns(scalar <>)) {
    say $name;
}


Reading a pointer to a string What it is doing basically is reading an std::string from a given pointer.

I was told that using a StringBuilder as I am is not the best way to achieve this, so I would like to know how this would be best written.

As additional information, that is a pointer of a std::string and I've made that because I was originally curious if there was a way to read a std::string in C# without having to create a bridge using C++/CLI.

How could I optimize this bit of code for best performance?

public static string ReadStdString(IntPtr address)
{
    var capacity = Marshal.ReadInt32(address + 24);
    var length = Marshal.ReadInt32(address + 20);
    var startStr = address + 4;
    if (capacity > 15)
        startStr = (IntPtr)Marshal.ReadInt32(address + 4);

    var result = new StringBuilder();
    for (var i = 0; i < length; i++)
    {
        result.Append((char)Marshal.ReadByte(startStr, i));
    }
    return result.ToString();
}

var result = new StringBuilder();
for (var i = 0; i < length; i++)
{
    result.Append((char)Marshal.ReadByte(startStr, i));
}

You're working in a tight loop: a StringBuilder looks like a reasonable tool to use.

One thing I would change that could impact performance (depending on the length of the string involved), is the StringBuilder constructor being used:

var result = new StringBuilder(length);

There's no reason not to specify the length of the string you're building if you know it from the start; that will reduce the overhead, since the internals of the builder won't need to manage growth.

The loop is a one-liner. You have this one-liner just a few instructions above:

if (capacity > 15)
    startStr = (IntPtr)Marshal.ReadInt32(address + 4);

Why does the loop have an explicit { } scope, but not the if block? It would be better to be consistent about scoping braces, and have them everywhere:

 if (capacity > 15)
 {
     startStr = (IntPtr)Marshal.ReadInt32(address + 4);
 }

An alternative to the StringBuilder could be to write the bytes into a MemoryStream:

using (var stream = new MemoryStream(length))
{
    for (var i = 0; i < length; i++)
    {
        stream.WriteByte(Marshal.ReadByte(startStr, i));
    }
    return new StreamReader(stream).ReadToEnd();
}

The nice thing with this approach is that you don't need to cast every single byte of the string into a char. On the other hand, you need 2 objects instead of one, and you need to cleanly Dispose of the stream, too - depending on the length of the string, the overhead might just not be worth it, although my guts tell me the stream could be faster... but I think you'd need to race your horses to find out.


Multiple filters and maps java stream I need to find the odd occurring numbers as a list. For example the below list should give 1 and 2 as output.
Is there a way that I can do it in a better way?
ArrayList<Integer> numbers = new ArrayList<>(Arrays.asList(1,2,2,4,2,4));
List<Integer> OddOcuranceNumbers =
numbers.stream()
.collect(Collectors.groupingBy(Function.identity(), Collectors.counting()))
.entrySet().stream()
.filter(map -> map.getValue() % 2 == 1).map(s -> s.getKey())
.collect(Collectors.toList());
System.out.println(OddOcuranceNumbers);
    }

That depends on how you define better. Better might be more CPU or memory efficient, or it could be fewer bytes in the compiled code, or better parallelization.
But I am going to take a chance on assuming that this code runs on a reasonably normal platform and has no special requirements, and therefore being better generally means better readability.
Readability Improvements
In which case, your code is fine. But we can make it easier to read by splitting it into named steps and wrapping it in a good function.
public List<Integer> getNumbersOccuringOddTimes(final List<Integer> numbers) {
    final Map<Integer, Integer> numberCounts = numbers.stream().collect(groupingBy(Function.identity(), counting()));
    final Predicate<Entry> isOdd = e -> e.getValue() % 2 == 1;
    return numberCounts.entrySet().stream().filter(isOdd).map(Entry::getKey).collect(toList());
}

I've made five minor adjustments to your code to help with readability. I have;

Added a descriptive method name that wraps the steps.
Split the two streams into separate statements.
Used static imports (not shown) for the Collectors functions.
Moved the modulus check into a predicate variable (isOdd) so it can be named.
Swapped out the lambda in .map to a method reference because it is shorter and more descriptive.


Zero-initializing large dynamically allocated arrays of double Arrays of double can be zero-initialized with the { 0 } initializer, which works even on systems that have a binary representation of 0.0 that is not all bits zero. While the IEEE 754 representation of floating point zero is all bits zero, the C Standard makes no guarantees about the representation of floating point numbers.

Large dynamically allocated arrays of integer types can be zero-initialized by  using calloc(), but this method is not portable for floating point types, since calloc() initializes all bits to zero. Assigning a value of 0.0 to each individual double in the segment using a loop seems grossly inefficient.

I wrote the code for the function calloc_d() to solve this problem. The function takes the size_t argument nmemb, and returns a zero-initialized segment of memory large enough to hold nmemb doubles. If there is an allocation error, the function returns NULL. The caller is of course responsible for deallocation.

After using malloc() to allocate the required amount of memory, the first bytes are assigned the value of 0.0. Then memmove() is used to copy the bytes of the first zero, then the bytes of the first two zeros, and so on until at least half of the memory has been initialized. Finally, the remaining memory is initialized to 0.0 by copying bytes from the first half.

The code works, and has no obvious issues, as far as I can tell. I am interested in any comments about shortcomings of this approach, or shortcomings of my code, and suggestions for improvement. I would also be interested in suggestions for alternative, possibly more efficient methods.

I have included the function in a working program below. MAXCOUNT doubles are allocated for, initialized, and displayed. The value of MAXCOUNT is currently set to 100, but I have tested it for values up to 1000000.

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

#define MAXCOUNT 100      // number of doubles to allocate for

double * calloc_d(size_t nmemb);

int main(void)
{
    double *dbl_store = calloc_d(MAXCOUNT);
    size_t i;

    for(i = 0; i < MAXCOUNT; i++) {
        if (i % 10 == 0) putchar('\n');
        printf("%-8.4f", dbl_store[i]);
    }
    putchar('\n');

    free(dbl_store);

    return 0;
}

double * calloc_d(size_t nmemb)
{
    double *ret;
    double *next;         // pointer to beginning of uninitialized segment
    size_t alloc_bytes = sizeof(*ret) * nmemb;
    size_t init_sz;       // size of initialized segment

    ret = malloc(alloc_bytes);
    next = ret;

    if (ret) {
        ret[0] = 0.0;
        init_sz = sizeof(*ret);
        ++next;

        while (init_sz < (alloc_bytes + sizeof(*ret)) / 2) {
            memmove(next, ret, init_sz);
            init_sz *= 2;
            next = ret + init_sz / sizeof(*ret);
        }
        memmove(next, ret, alloc_bytes - init_sz);
    }

    return ret;
}

So my first hunch when I read your question is:

Okay this guy is making a complicated solution to a simple problem based on a hunch that the trivial solution is "grossly inefficient" without any measurements to back up said claim. I suspect OPs solution is slower than the trivial solution.

So I set out test this, I pit your fancy implementation against my trivial implementation:
double * calloc_d2(size_t nmemb){
    double* ret = malloc(sizeof(double)*nmemb);
    double* end = ret + nmemb;

    if(ret){
        double* i = ret;
        while(i != end){
            *i = 0.0;
            ++i;
        }
    }
    return ret;
}

I tested this here on ideone.com (I think it is using clang) which shows that this simple bit of code is consistently around 3-5% faster than your fancy code.
So I have two pieces of advice

Express your intent as clearly as possible, and let the compiler worry about generating high performance code. Compilers are pretty smart...
Always, always, always measure.

Addendum/Edit
Seeing as this answer has gotten quite some attention I've gone ahead and dug deeper.
Using godbolt compiler explorer I compiled (-O3) and disassembled my trivial loop implementation.
Here is the disassembly on GCC 6.2:
calloc_d2(unsigned long):
        push    rbp
        push    rbx
        lea     rbx, [0+rdi*8]
        sub     rsp, 8
        mov     rdi, rbx
        call    malloc
        test    rax, rax
        mov     rbp, rax
        je      .L1
        lea     rax, [rax+rbx]
        cmp     rbp, rax
        je      .L1
        mov     rdx, rbx
        xor     esi, esi
        mov     rdi, rbp
        call    memset
.L1:
        add     rsp, 8
        mov     rax, rbp
        pop     rbx
        pop     rbp
        ret

Notice how the compiler has just gone "Oh, you want to zero that? Cool, I'll just replace your code with a memset". This goes to show that an optimising compiler is quite smart and is able to transform your code in astounding ways.
I also tried different versions of GCC and clang on ARM and MIPS and PowerPC, they all generate similar instructions which essentially boil down to call malloc then memset.
With the sole exception of ICC 17 (-O3 -march=native) which goes above and beyond:
calloc_d2(unsigned long):
        push      rbp                                           #5.1
        mov       rbp, rsp                                      #5.1
        and       rsp, -32                                      #5.1
        push      r15                                           #5.1
        push      rbx                                           #5.1
        sub       rsp, 16                                       #5.1
        mov       rbx, rdi                                      #5.1
        lea       rdi, QWORD PTR [rbx*8]                        #6.50
        call      malloc
        -- snip --
        call      __intel_avx_rep_memset                        #12.8
        -- snip --

Notice that it calls into __intel_avx_rep_memset instead, this is a 512 bit wide AVX optimized memset that will only function on modern CPUs (it did this because I passed -march=native asking it to throw backward compatibility out the window). There is some branching code at the end which deals with cases where nmemb isn't evenly divisible by 512/64=8.
But here is the key take away, we clearly expressed our intent with the trivial loop. The compiler could then replaced the trivial loop (after deducing it's functionality) with an (512 bit) AVX optimised memset written by Intel's engineers specifically for modern Intel CPUs.
There is this thing called the "as-if rule" in C/C++. Which pretty much says:

As long as the program does the same reads and writes to volatile memory in the same order as-if the program was executed according to the language specification, then the program may do ANY transformation of the code it pleases. Including reordering instructions, reordering stores and loads to non-volatile memory, or removing instructions all together.

Just to show you how awesome some compilers can be look at what clang does here:
auto stupid(){
   auto x = new int[30];
  for(int i = 0; i < 30; i++)
    x[i] = i;

  auto sum = 0;
  for(int i = 0; i < 30; i++)
    sum += x[i]*x[i];
  delete [] x;
  return sum;
}

gets compiled into:
stupid():                             # @stupid()
        mov     eax, 8555
        ret

It deduces that the function always returns the same value and it completely removes all code and just returns a constant! It also removes the calls to new and delete as it realises they are pointless and don't affect the output of the function. If that is not impressive, I don't know what is. :)
So I would like to take this opportunity to reiterate my first piece of advice again:
Clearly express what it is you want your code to do. Let the compiler worry about generating good machine code, the compiler is likely much better at it than you are.


Using datetime in order to name files I have a script that makes use of datetime in order to name files.

Only business days (Mon - Fri) are used.

Here is my part of the code that finds the last business day:

lastBusDay = datetime.datetime.today()
shift = datetime.timedelta(max(1,(lastBusDay.weekday() + 6) % 7 - 3))
lastBusDay = lastBusDay - shift
yesterday = lastBusDay.strftime('%d%m%y')

These variables are outside of any function and are the first part of my script after the imports. I've heard that global variables are bad, and I understand why.

But does this fall under the same category?

Only the yesterday variable is used later on in the script and is used to specify the new name in an the os.renames function.

I think you only want to calculate the last working day from today. Here is a better/pythonic approach:

from datetime import timedelta, date

def lastBzDay( d = None ):
  r, d = xrange( 1, 5 ), d or date.today()
  def retfn( curDay = d ):
    w = curDay.weekday()
    prevBzDay = curDay - timedelta( days= 1 if w in r else (w + 3) % 7 )
    return prevBzDay.strftime('%d%m%y')
  return retfn
lastBzDay = lastBzDay()

Now, all you have to do is:

yesterday = lastBzDay()

and you'll get the desired result. Using a closure also gives you the ability to find the last weekday from any other dates:

print lastBzDay( date(2014, 9, 7) )  # will give you 050914

You can see a demo on codepad here.


Select records between the first and last time the speed was positive I have the following table:

id   speed   date
1    0       01/01/2015
2    0       01/01/2015
3    0       01/01/2015
4    0       01/01/2015
5    0       01/01/2015
6    0       01/01/2015
7    25      01/01/2015
8    78      01/01/2015
9    13      01/01/2015
10   45      01/01/2015
11   0       01/01/2015
12   80      01/01/2015
13   86      01/01/2015
14   10      01/01/2015
15   0       01/01/2015
16   0       01/01/2015
17   0       01/01/2015
18   0       01/01/2015

This is just a small part of the table, it has many dates. Each new date starts and ends with an unknown amount of zeros. And I have to skip these zeros and get only what's between them. So I came up with the following solution:

Get the ID of the first row whose speed is positive (in our table it's id number 4).
Get the ID of the last row whose speed is positive (in our table it's id number 12).
Get lines between these two IDs (4 and 12).

I get the id of the first row whose speed is positive:

$q1=$conn->prepare("SELECT * FROM tableX WHERE speed > 0 order by date asc LIMIT 1");
$q1->execute();
$r1=$q1->fetch();
$first_id = $r1['id'];

And then get the id of the last row whose speed is positive:

$q2=$conn->prepare("SELECT * FROM tableX WHERE speed > 0 order by date desc LIMIT 1");
$q2->execute();
$r2=$q2->fetch();
$last_id = $r2['id'];

And then use those ids to get what I want:

$q3=$conn->prepare("SELECT * FROM tableX WHERE id between '$first_id' and '$last_id'");
$q3->execute();
while($r3=$q3->fetch(){}

Expected result:

id   speed   date
7    25      01/01/2015
8    78      01/01/2015
9    13      01/01/2015
10   45      01/01/2015
11   0       01/01/2015
12   80      01/01/2015
13   86      01/01/2015
14   10      01/01/2015

My code is actually working but I think this solution of mine is lame, so I am looking for an improvement, kind of all these three queries in one!

First of all a style comment: Please use longer descriptive names. Stuff like $q3, $q2, is just ugly, and confusing.

You could simplify your query either by using subqueries in the WHERE clause, or by creating a temporary table in the FROM clause. You could use the BETWEEN operator, but some think BETWEEN is evil as it is somewhat unclear from the wording whether the range is inclusive or exclusive. In addition if used on date fields, you could have various interesting rounding errors. So it better to stick with ordinary operators.

I've commented upon the fact that you'll have a potential issue related to order, and I haven't compensated for that in the following code, only assuming that rows are indeed ordered by id. This could/should possibly be enforced. Here is the SQL code to get your rows in one go:

SELECT tableX.* FROM tableX,
  ( SELECT min(id) as startId, max(id) as endId FROM tableX
     WHERE speed > 0 AND date = '2015-01-01' ) tmp
 WHERE id >= tmp.startId AND id <= tmp.endId

Note that in the temporary table select I chosen a given date, instead of ordering by date (which most likely would throw of the id part quite easily. Another option could be to specify a double order like in ORDER BY date, id.

Untested version translated into php-code:

$speed_query = $db_connection->prepare(
   "SELECT tableX.* FROM tableX,
     ( SELECT min(id) as startId, max(id) as endId FROM tableX
        WHERE speed > 0 AND date = '$date' ) tmp
   WHERE id >= tmp.startId AND id <= tmp.endId ");
$speed_query->execute();
while($row = $speed_query->fetch()) {
   ... do something useful with $row ...
}

The advantage of this method is that you now get all of your data from one single query, and one roundtrip from your code base to the database engine. Using subqueries like this shouldn't be to expensive, and at least when comparing to making several roundtrips it is a way better solution.

PS! You could consider using heredocs or other variations to get the
multiline string in your query. But do try to break it over multiple lines
to enhance the readability of the SQL query. See
PHP string doc for
various option on how to use strings in PHP.


Generate array with random numbers and search for an element in it This is a school project I've written. It works, but I feel it could be done better.

The program generate an array of 1'000 random numbers, and searches for a number typed into console, and displays the index if found.

int inp, zuf, i, num, trig=0;
const int amount = 1000;
int arr[amount];
cout << "Numbers" << endl << "======" << endl << endl;
cout << "Please type in a nuber between 1 and 1000: " << endl;
cin >> inp;
srand(time(NULL));
for (int i = 1; i < amount + 1; i++) {
    zuf = rand();
    zuf = (zuf % amount) + 1;
    arr[i] = zuf;
    num = i;

    if (inp== arr[num] && trig == 0) {
        cout << "The number exists at the position: ";
        trig++;
    }
    if (inp== arr[num]) {
        cout << num << ", ";
        num++;
        trig++;
    }
}

if (trig == 0) cout << "This number doesn't exist." << endl;
else cout << "\b\b." << endl;

Do not using using namespace std

bringing in entire namespaces can be convenient, but in the long run, it's a really bad habit.

You should explicitly use std::cout and the like instead.

Declare variables as close as possible to their first use.

int inp, zuf, i, num, trig=0;

This means very little out of context.

Prefer using std::array instead of regular c-style arrays.

std::array<int, amount> arr;

This plays better with the rest of the standard.

Do not use 1-based indexing unless you really need it.

And in your case, you definitely do not want it, as you are doing an out-of-bounds access on arr[i] when i == amount;

for (int i = 1; i < amount + 1; i++) {

should be

for (int i = 0; i < amount; i++) {

But it doesn't matter because:

prefer using ranged-based for unless necessary

for(auto& val : arr) {
  val = ...
}

Do not use srand()/rand()

They are terrible. It's slow and not particularly random. You should use the random number genreator provide by <random> instead:

https://en.cppreference.com/w/cpp/numeric/random/uniform_int_distribution.

Separate your concerns:

I suspect you are not respecting the "spirit of the exercise", since filling the array doesn't serve a purpose in your example.

You probably want to first fill the array, and then ask the user (possibly multiple times) to provide a number to find.


AJAX-based PHP server-side form validation I am still a newbie at coding in general. Over the years the amount of programming that I have done in PHP has always been procedural. I am now working on a project for my course, and was hoping to use it as an opportunity to learn OOP since I come across it everywhere.

I am still trying to really get my head around OOP. I understand the basics (I hope).

Here is my very first attempt. I am working on an AJAX based server-side (PHP) form validation. I have tried to write my validation code along OOP principles.

Does this make sense from an OOP perspective?

    require_once 'db.php';

class Registree {

    /*Some of the properties*/
    var $Username;
    var $Email;
    var $Password;
    var $Password_repeat;

    /*Function to check that the $ProposedUsername is not already taken*/
    function checkUsername($ProposedUsername) {

        $this->$username = $ProposedUsername;

        /*Only start checking once the user has entered 4 or more characters into the username field*/
        if (strlen($ProposedUsername) => 4) {

            /*Make a query from the users table based on the $ProposedUsername*/
            $result = mysqli_query($dblink, "SELECT * FROM users WHERE `username` = '$ProposedUsername'")
            or die(mysqli_error($dblink));

            /*If any rows were affected, then this username is taken*/
            if (mysqli_affected_rows($dblink)) > 0) {
                echo 'Username already taken';
            } else {
                    echo 'Username available';
                }
        }
    }
}

What I would do:

Use an autoloading function (basic example, PHP manual) to get rid of require_once
$this->$username won't work as you expect, replace it with $this->Username
Avoid magic numbers like if (strlen($ProposedUsername) => 4), instead define constants (or class constants)

But basically, it seems that you have quite well understood the basic principles of OOP.

As a matter of preference, I would:

Use Allman coding style
Use // instead of /* */ for one line comments
throw a UsernameAlreadyTakenException (create it by simply extending Exception) and do nothing if username is available, instead of simply echoing the checking results.
EDIT: I like exceptions for such "checking" functions, because IMO it's a good way to refactor tons of "if" + error handling code. Calls look like:

// clean readable checkings, and if something goes wrong, you are informed :-)
// however and obviously, in production environments, all exceptions should be
// caught at high level ==> logged + a user-friendly message displayed.
$user->checkUsernameExists();
$user->checkEmailExists();

// or in a few cases
try {
  $user->checkUsernameExists();
} catch (UsernameAlreadyExistsException $e) {
  //...
}


Finding count of character occurrences in a string I am taking an introductory course in java in high school, however I have some experience in python. I was tasked to find the number of times a character occurs in a string so I did this.

String a = "hello";
String letter = "l";
int occurrence = a.length() - a.replace(letter, "").length();

Happy that I came up with a clever one-liner, I showed it to my teacher and she said that it's more elegant to use a for loop. What does she mean by that? How do we define elegance in code? Is it subjective? As an aside is this more efficient than a for loop or vice-versa?

I am not sure I agree that a for loop is more elegant per se. The downside of your method compared to a traditional for loop is that it requires a bit more processing most of the time, because you need to create a new character buffer and put the replacements in there, and you create a new String object.

I would probably create an initial implementation with a Java 8 Stream, e.g.:

String string = "hello";
char letter = 'l';
long count = string.chars().filter(character -> character == letter).count();

To me this is probably more elegant than a traditional for loop. With Streams it is possible to write it in a declarative way. In a lot of cases they are also trivial to parallelize, requiring only a parallel function call on the Stream. Furthermore, they are lazy which means the processing can stop early, only the necessary data will be processed. Note that in this case, we have to process each element (each character), so it makes no difference here.


Saving a collection of WPF user control layout into an XPS document I am making an application to save a layout of user control into an .xps document. But, the export doesn't seem completed if the data is too many. I've tried to encapsulate each part with DispatcherPriority.Loaded but it seems the data binding is horrifyingly take too long to be finished (I honestly don't know what is the problem to be exact).

How can I optimize the below code properly to wait for each process to be finished before moving on to the next one, so saving to XPS document will be properly loaded?

P.S. This doesn't happen if the data is not too large.

public class MyClass
{
    public static void CreatePortableFile(List<MyViewModelVM> myViewModels, string path)
    {
        List<MyViewV> views = new List<MyViewV>();
        List<FixedPage> fixedPages = new List<FixedPage>();
        List<PageContent> pageContents = new List<PageContent>();
        FixedDocument fixedDoc = new FixedDocument();

        Dispatcher.CurrentDispatcher.Invoke(new Action(() =>
            {
                foreach (MyViewModelVM item in myViewModels)
                {
                    views.Add(new MyViewV() { DataContext = item });
                }
                Console.WriteLine("Setting datacontext " + DateTime.Now.TimeOfDay);
            }), DispatcherPriority.Loaded);

        Dispatcher.CurrentDispatcher.Invoke(new Action(() =>
            {
                foreach (MyViewV item in views)
                {
                    FixedPage newFixedPage = new FixedPage();
                    newFixedPage.Children.Add(item);
                    fixedPages.Add(newFixedPage);
                }
                Console.WriteLine("Setting fixedpage " + DateTime.Now.TimeOfDay);
            }), DispatcherPriority.Loaded);

        Dispatcher.CurrentDispatcher.Invoke(new Action(() =>
            {
                foreach (FixedPage item in fixedPages)
                {
                    PageContent newPageContent = new PageContent();
                    ((System.Windows.Markup.IAddChild)newPageContent).AddChild(item);
                    pageContents.Add(newPageContent);
                }
                Console.WriteLine("Setting pagecontent " + DateTime.Now.TimeOfDay);
            }), DispatcherPriority.Loaded);

        Dispatcher.CurrentDispatcher.Invoke(new Action(() =>
            {
                foreach (PageContent item in pageContents)
                {
                    fixedDoc.Pages.Add(item);
                }
                Console.WriteLine("Setting fixeddoc " + DateTime.Now.TimeOfDay);
            }), DispatcherPriority.Loaded);

        Dispatcher.CurrentDispatcher.Invoke(new Action(() => WriteToXps(path, fixedDoc)), DispatcherPriority.Loaded);
    }

    private static void WriteToXps(string path, FixedDocument fixedDoc)
    {
        XpsDocument xpsDoc = new XpsDocument(path, FileAccess.Write);
        XpsDocumentWriter xWriter = XpsDocument.CreateXpsDocumentWriter(xpsDoc);
        xWriter.Write(fixedDoc);
        xpsDoc.Close();

        Console.WriteLine("Setting write " + DateTime.Now.TimeOfDay);
    }
}

Sample call:

MyClass.CreatePortableFile(listOfMyViewModels, path);

Console.Writeline result:

Setting datacontext 17:51:37.8885505
Setting fixedpage 17:51:37.8915518
Setting pagecontent 17:51:37.8935523
Setting fixeddoc 17:51:37.8945527
Setting write 17:51:40.1266807

I don't really see the sense of splitting this into different sections. Also some of them only add overhead which isn't needed at all.

For each MyViewModelVM in List<MyViewModelVM> you are creating a MyViewV with it as DataContext which is in the next loop added as a child to a FixedPage which then is added as a child to a PageContent which is then added to the Pages of the FixedDocument.

This can be simplified to

public static void CreatePortableFile(List<MyViewModelVM> myViewModels, string path)
{

    Dispatcher.CurrentDispatcher.Invoke(new Action(() =>
    {
        FixedDocument fixedDoc = new FixedDocument();

        foreach (MyViewModelVM item in myViewModels)
        {
            MyViewV view = new MyViewV() { DataContext = item };

            FixedPage newFixedPage = new FixedPage();
            newFixedPage.Children.Add(view);

            PageContent newPageContent = new PageContent();
            ((System.Windows.Markup.IAddChild)newPageContent).AddChild(newFixedPage);
            pageContents.Add(newPageContent);

            fixedDoc.Pages.Add(newPageContent);
        }
        WriteToXps(path, fixedDoc)

    }), DispatcherPriority.Loaded);
}

EDIT After reading the comment, I digged a little bit more into this. As this is similiar to printing with datacontext this should fix the issue:

public static void CreatePortableFile(List<MyViewModelVM> myViewModels, string path)
{

    FixedDocument fixedDoc = new FixedDocument();

    foreach (MyViewModelVM item in myViewModels)
    {
        MyViewV view = new MyViewV() { DataContext = item };

        FixedPage newFixedPage = new FixedPage();
        newFixedPage.Children.Add(view);

        PageContent newPageContent = new PageContent();
        ((System.Windows.Markup.IAddChild)newPageContent).AddChild(newFixedPage);
        pageContents.Add(newPageContent);

        fixedDoc.Pages.Add(newPageContent);
    }

    Dispatcher.CurrentDispatcher.Invoke (new Action (delegate { }), DispatcherPriority.ApplicationIdle, null);

    WriteToXps(path, fixedDoc)
}

A detailed explaination can be found here


How to optimize these nested loops for better performance? I need to optimize this code so it can execute faster, even if that means using more memory:

for (int t = 0; t < Clientes[0].ChildNodes.Count; t++)
{
    for (int i = 0; i < Clientes[0].ChildNodes.Item(t).ChildNodes.Count; i++)
    {
        if (Clientes[0].ChildNodes.Item(t).ChildNodes.Item(i).Name.Contains("Service"))
        {
            int a = 0;
            int.TryParse(Clientes[0].ChildNodes.Item(t).ChildNodes.Item(i).Name.ToUpper().Replace("SERVICE", ""), out a);
            if (a.Equals(Type))
            {
                Queues[t].Send(QueueMessage);
            }
        }
    }
}

I have decided to avoid accessing Clientes[0] and its ChildNodes.Item(t) over and over in each loop by keeping them in new variables like this:

XmlNodeList clientZeroChildNodes ;
int clientZeroChildCount ;

XmlNodeList clientZeroItemTchildNodes ;
int clientZeroItemTchildCount;
string clientZeroItemTchildIname;

clientZeroChildNodes = Clientes[0].ChildNodes;
clientZeroChildCount = clientZeroChildNodes.Count;

for (int t = 0 ; t < clientZeroChildCount ; t++)
{
    clientZeroItemTchildNodes = clientZeroChildNodes.Item(t).ChildNodes;
    clientZeroItemTchildCount = clientZeroItemTchildNodes.Count;

    for (int i = 0; i < clientZeroItemTchildCount; i++)
    {
        clientZeroItemTchildIname = clientZeroItemTchildNodes.Item(i).Name;

        if (clientZeroItemTchildIname.Contains("Service"))
        {
            int a = 0;

            int.TryParse(clientZeroItemTchildIname.ToUpper().Replace("SERVICE", ""), out a);

            if (a.Equals(Type))
            {
                try
                {
                    Queues[t].Send(QueueMessage);
                }
                catch (MessageQueueException mqe)
                {
                    //log error about mqe
                }
            }
        }
    }

}

As you can see, the second for loop only takes certain items of the ChildNodes based on its name. I have tried to use LINQ so that the second for only iterates the list of ChildNodes that match both of the if conditions that are inside the loop but have not been able to do it.

Can you show me how would you improve the speed of the original code focusing on an efficient way to improve the loops and nested ifs? I am not trying to determine which line of code is taking longer and the performance of the Queues is not relevant for me.

it looks like you are doing a lot with making all sorts of variables that are really confusing to look at, maybe I am simple minded and that is why I can't figure it all out.

not really sure what you are doing with the t  but I changed it to i

 XmlNodelist ClientNodes;
ClientNodes = Clientes[0].ChildNodes;
XmlNode ChildNode;

int i = 0;

foreach ChildNode in ClienteNodes
{
    i++
    XmlNodeList ClientChildNodes;
    XmlNode ClientChildNodeChild;  //don't keep my name scheme
    foreach ClientChildNodeChild in ClientChildNodes
    {
        string clientZeroItemTchildIname;
        clientZeroItemTchildIname = (ClientChildNodeChild.Item.Name).ToUpper();
        if ( ClientZeroItemTchildIname.Contains("SERVICE"))
        {
            int a = 0;
            int.TryParse(ClientZeroItemTchildIname.Replace("SERVICE",""),out a);

            if (a.Equals(Type))
            {
                try
                {
                    Queues[i].Send(QueueMessage)
                }
                catch (MessageQueueException mqe)
                {
                    //log Error about mqe
                }
            }
        }
    }
}

not sure that I made it any faster, but this is way more readable.

there is one less Variable in my code than there is in your 'optimized' code.

Which is faster,  foreach or a for loop?

Here is an answer to that question, where someone actually went through testing for loops against foreach loops.  and it looks like the foreach is faster in some instances.

Answer to Performance difference for control structures 'for' and 'foreach' in C# (2009)

I also found a page where someone claims that a foreach loop takes longer and is generally good for collections, but then he recommends against it anyway.

Code Project Article "FOREACH Vs. FOR (C#)" (2004)

Threw that one in there in case you thought that I was biased.  I still think the foreach would be better for readability, but I don't know what your code is doing so I don't know what is going to be more efficient for you.  Readable/maintainable vs. faster?  maybe.  not sure though.

Something else to think about

Take the Send out of the Loop.

keep track of which Queues that you want to send in a variable outside of the loop and then once you are finished looping then send them all at the same time, outside of the loop.


Program for string length in x86 It takes in a string from the command line. prints out the length of the string.
I'm mostly looking for tips and general convention suggestions
But I'm open to any other criticism also.
Roast it please thanks.

global _start

extern printf

section .data
    arg_err db `Invalid Arg Length\n\0`
    len_arg_err equ $ - arg_err
    arg_frmt db `%d\n\0`

section .text
_start:
    mov ecx, DWORD [esp]
    cmp ecx, 2
    jne _error_ext ; Args != 2?

    mov eax,DWORD [esp+8] ;char** argv
    mov ecx, eax

_label:
    mov dl, byte [ecx]
    cmp dl, 0; read byte and comp to 0
    je _exit
    add ecx, 1
    jmp _label

_exit:
    sub ecx,eax ; subtract end addr from start addr
    push ecx
    push arg_frmt
    call printf
    mov eax, 1
    int 0x80

_error_ext:
    mov ecx, arg_err
    mov edx, len_arg_err
    call _print
    mov eax, 1
    int 0x80

_print: ; needs ecx as char* edx as char* len

    push eax
    push ebx

    mov eax, 4
    mov ebx, 1
    int 0x80
    pop ebx
    pop eax
    retn

mov dl, byte [ecx]
cmp dl, 0; read byte and comp to 0
je _exit

There are various alternatives, for example:

cmp byte [ecx], 0
je _exit

And:

mov dl, [ecx] ; note: BYTE would be redundant, but you can put it if you like
test dl, dl
jz _exit

For that loop overall,

_label:
    mov dl, byte [ecx]
    cmp dl, 0; read byte and comp to 0
    je _exit
    add ecx, 1
    jmp _label

This is a "2 jump loop", but it can be a "1 jump loop", for example:

_label:
    mov dl, [ecx]
    inc ecx
    test dl, dl
    jnz _label

Or:

    dec ecx
_label:
    inc ecx
    cmp byte [ecx], 0
    jne _label

(here are some details about inc vs add)

For fast loops like this, having two jumps in the loop can easily slow them down to half the speed.

repnz scasb is an option, but it's not that great. Small loops like this can run in 1 cycle per iteration on most CPUs anyway, and repnz scasb is not better than that (often worse, such as 2 cycles per byte on Ryzen and Haswel). The special optimizations for rep movs and rep stos don't apply to rep scas.

There are faster ways to implement strlen, that are not based on byte-by-byte loops but detect the presence of a zero inside a bigger block, using SIMD (example) or SWAR (example). That way speeds much higher than 1 byte per cycle can be reached. Perhaps some future processor would implement a "fast rep scas" feature in a similar way..

mov ecx, arg_err
mov edx, len_arg_err
call _print
int 0x80

This is mysterious, there is no _print function and there is a stray system call without selecting any particular entry point, or perhaps _print was meant to put a suitable value in eax?


Codility's stonewall 100% solution - different paths such as if,else if,etc, do the same thing Here is the task description for the codility's stonewall problem. I would like to get a feedback on my code, and I specify at the bottom what the title states. (I had to make the figure by hand, because I couldn't paste the site's one for some reason, but it can be seen here, if mine doesn't look right)

  Task description

  You are going to build a stone wall. The wall should
  be straight and N meters long, and its thickness should be constant;
  however, it should have different heights in different places. The
  height of the wall is specified by an array H of N positive integers.
  H[I] is the height of the wall from I to I+1 meters to the right of
  its left end. In particular, H[0] is the height of the wall's left end
  and H[N−1] is the height of the wall's right end.

  The wall should be built of cuboid stone blocks (that is, all sides of
  such blocks are rectangular). Your task is to compute the minimum
  number of blocks needed to build the wall.

  Write a function:

  class Solution { public int solution(int[] H); }

  that, given an array H of N positive integers specifying the height of
  the wall, returns the minimum number of blocks needed to build it.

  For example, given array H containing N = 9 integers:

  H[0] = 8 H[1] = 8    H[2] = 5   H[3] = 7
    H[4] = 9 H[5] = 8    H[6] = 7   H[7] = 4    H[8] = 8
  the function should return 7. The shows one possible arrangement of seven
  blocks.

  Write an efficient algorithm for the following assumptions:

  N is an integer within the range [1..100,000]; each element of array H
  is an integer within the range [1..1,000,000,000].

And here is my 100% O(N) time complexity solution.
As the title says, I find it kinda weird that different paths do the same thing. Should I fix that? if so... how? the logic is correct. And should I care? Thinking in bigger problems, if many paths do the same thing, and requirements change, I should change code in many places, which is not recommended.

using System;
using System.Collections.Generic;
using System.Linq;

class Solution {
    public int solution(int[] H) {
        Stack<int> heightsStack = new Stack<int>();
        int blocksCounter = 0;

        for (int height = 0; height < H.Length; height++)
        {
            if (heightsStack.Count > 0)
            {
                while (heightsStack.Count > 0 && heightsStack.Peek() > H[height])
                {
                    heightsStack.Pop();
                }
                if (heightsStack.Count > 0)
                {
                    if (heightsStack.Peek() < H[height])
                    {
                        blocksCounter++;
                        heightsStack.Push(H[height]);
                    }
                }
                else
                {
                    blocksCounter++;
                    heightsStack.Push(H[height]);
                }
            }
            else
            {
                blocksCounter++;
                heightsStack.Push(H[height]);
            }
        }
        return blocksCounter;
    }
}

Some of these branches can be eliminated. Let's see:

if (heightsStack.Count > 0)
{
  if (heightsStack.Peek() < H[height])
  {
    blocksCounter++;
    heightsStack.Push(H[height]);
  }
}
else
{
  blocksCounter++;
  heightsStack.Push(H[height]);
}

Here, there are two conditions where we execute the two common lines: heightsStack.Count <= 0 or heightsStack.Peek() < H[height]. Since Count  can't be negative, let's rewrite it as such:

if (heightsStack.Count == 0 || heightsStack.Peek() < H[height])
{
  blocksCounter++;
  heightsStack.Push(H[height]);
}

if (heightsStack.Count > 0)
{
  while (heightsStack.Count > 0 && heightsStack.Peek() > H[height])
  {

The first if-statement is redundant. If heightStack.Count > 0 is false, the while statement will be false as well. We would immediately fall through and end up at the if-statement in the section above. This if-statement would evaluate to true, since heightStack.Count == 0 still. And it would execute the two common lines.

This would leave the body of the for-loop as follows:

while (heightsStack.Count > 0 && heightsStack.Peek() > H[height])
{
  heightsStack.Pop();
}
if (heightsStack.Count == 0 || heightsStack.Peek() < H[height])
{
  blocksCounter++;
  heightsStack.Push(H[height]);
}

Note, that we only have one branch with the two common lines.

There's one last thing. You're using a for-loop, and only using the looped variable to index directly. Instead, consider using a foreach loop:

public int solution(int[] H)
{
  Stack<int> heightsStack = new Stack<int>();
  int blocksCounter = 0;

  foreach(int height in H)
  {
    while (heightsStack.Count > 0 && heightsStack.Peek() > height)
    {
      heightsStack.Pop();
    }
    if (heightsStack.Count == 0 || heightsStack.Peek() < height)
    {
      blocksCounter++;
      heightsStack.Push(height);
    }
  }
  return blocksCounter;
}

As far as algorithm goes, you do what I would have come up with, so I have nothing more to say about that.


Code snippet for a method dealing with IDs from multiple threads I have multiple threads calling a method, passing in an ID and value.

I have two constraints I need to place on this method:

Only one of the same ID can be processed at a time. Additional threads with the same ID must wait.
Subsequent calls with the same ID must be processed in the order they were received.

Here's an Example.

Method is called by four different threads with the parameters

ID 1 Value a
ID 2 Value b
ID 1 Value c
ID 1 Value d

Program should be able to process [ID 1 Value a] and [ID 2 Value b] immediately, * while [ID 1 Value c] and [ID 1 Value d] wait for [ID 1 Value a] to finish.
Once [ID 1 Value a] finishes, then the method runs for [ID 1 Value c] while [ID 1 Value d] waits.
When [ID 1 Value c] finishes the method runs for [ID 1 Value d].
If the method wrote the value to the console then I'd expect the output to be either a, b, c, d or b, a, c, d.

I've had a quick go at it but it's messy. I'm writing this post in hopes of getting an improved and abstracted code snippet for doing this.

Here is my attempt:

static object _lockObj = new object();
static Dictionary<int, Queue<ManualResetEvent>> runningIds = new Dictionary<int, Queue<ManualResetEvent>>();

public async Task TheMethod(int id, string val)
{
    ManualResetEvent enquedResetEvent = null;
    lock (_lockObj)
    {
        if (runningIds.ContainsKey(id))
        {
            enquedResetEvent = new ManualResetEvent(false);
            runningIds[id].Enqueue(enquedResetEvent);
        }
        else
        {
            runningIds.Add(id, new Queue<ManualResetEvent>());
        }
    }
    if (enquedResetEvent != null)
        enquedResetEvent.WaitOne();
    try
    {
        // DO LONG RUNNING TASK
        await Task.Delay(3000);
        Console.WriteLine(id + " " + val + " at time " + DateTime.Now.ToString("m:ss"));
    }
    finally
    {
        lock (_lockObj)
        {
            if (runningIds[id].Count > 0)
            {
                var dequedResetEvent = runningIds[id].Dequeue();
                dequedResetEvent.Set();
            }
            else
            {
                runningIds.Remove(id);
            }
        }
    }
}

Sample input and output would be:

List<Task> ts = new List<Task>();
ts.Add(Task.Run(() => new Test().TheMethod(1, "a")));
Thread.Sleep(10);
ts.Add(Task.Run(() => new Test().TheMethod(2, "b")));
Thread.Sleep(10);
ts.Add(Task.Run(() => new Test().TheMethod(1, "c")));
Thread.Sleep(10);
ts.Add(Task.Run(() => new Test().TheMethod(1, "d")));
Task.WaitAll(ts.ToArray());
// Output:
// 1 a at time 25:53
// 2 b at time 25:53
// 1 c at time 25:56 // note 3 second delay
// 1 d at time 25:59
// 1 e at time 26:02
// 1 f at time 26:05

Here are my thoughts on the code that you provided:

You should mark your lockObj and runningIds as readonly
You are using the presence of the queue in the dictionary as a signal that another thread is currently working on specific ID which is not quite obvious
You mix non-blocking (await) approach along with blocking (ManualResetEvent.WaitOne). It would be better to stick to one of them (preferably non-blocking).
If you change the approach from "notify the guy after me that he can start working" to "I'm done", then you would be able to capture only the last "guy" against specific ID. "I'm done" is easily modelled by Task.

As a result, my first refactoring of your code looks like this:

private static readonly object _lockObj = new object();
private static readonly Dictionary<int, Task> _runningIds = new Dictionary<int, Task>();

public async Task TheMethod(int id, string val)
{
    //Will be used to signal others that the work has finished
    var completionSource = new TaskCompletionSource<bool>();

    Task currentTask;

    lock (_lockObj)
    {
        _runningIds.TryGetValue(id, out currentTask);
        _runningIds[id] = completionSource.Task;
    }

    if (currentTask != null)
        await currentTask;

    // DO LONG RUNNING TASK
    await Task.Delay(3000);
    Console.WriteLine(id + " " + val + " at time " + DateTime.Now.ToString("m:ss"));

    completionSource.SetResult(true);

    //cleanup
    lock (_lockObj)
    {
        if (_runningIds.TryGetValue(id, out currentTask) && currentTask == completionSource.Task)
            _runningIds.Remove(id);
    }
}

Second take was mostly as an exercise to get rid of lock at all by using the ConcurrentDictionary.

private static readonly ConcurrentDictionary<int, Task> _runningIds = new ConcurrentDictionary<int, Task>();

private static Task RegisterAsLastScheduledTask(int id, Task task)
{
    Task previousTask = null;

    //Either adding a new task to the dictionary, or capturing and replacing previous task
    while (!_runningIds.TryAdd(id, task) &&
        !(_runningIds.TryGetValue(id, out previousTask) && _runningIds.TryUpdate(id, task, previousTask)))
    {
        previousTask = null;
    }

    return previousTask;
}

public async Task TheMethod(int id, string val)
{
    //Will be used to signal others that the work has finished
    var completionSource = new TaskCompletionSource<bool>();
    Task previousTask = RegisterAsLastScheduledTask(id, completionSource.Task);

    if (previousTask != null)
        await previousTask;

    // DO LONG RUNNING TASK
    await Task.Delay(3000);
    Console.WriteLine(id + " " + val + " at time " + DateTime.Now.ToString("m:ss"));

    completionSource.SetResult(true);

    //"hack" to atomically remove the element only if both key and value match, see http://blogs.msdn.com/b/pfxteam/archive/2011/04/02/10149222.aspx
    ((ICollection<KeyValuePair<int, Task>>)_runningIds).Remove(new KeyValuePair<int, Task>(id, completionSource.Task));
}


Nested If, working like an excel SUMIF for two unequal lists summing the distance if GPS timestamp meets criteria This is my first post, I am very new to coding and Python especially,

This code intends to do an excel SUMIF between two tables with different indexes. The first tables has GPS data with timestamp, vehicle ID and distance The second table has vehicle ID and timestamps of events I want to measure the distance run during events

Thanks

for x in range(1,34):
df = pd.read_csv("file"
                 + str(x) + '.csv',
                 parse_dates=[10])

red = 0
green = 0
black = 0

output = [[], [], [], []]
for i in range(len(lista[1])):
    for j in range(len(listc[1])):
        if listc[1][j] <= lista[3][i] or listc[1][j] >= lista[2][i]:
            if lista[7][i] >= listc[1][j] and lista[6][i] <= listc[1][j] and lista[0][i] == listc[0][j] and lista[8][i] == 'intended value' :
                red += listc[2][i]
            if lista[3][i] >= listc[1][j] and lista[7][i] <= listc[1][j] and lista[0][i] == listc[0][j] and lista[8][i] != 'intended value' :
                red += listc[2][i]
            if lista[6][i] >= listc[1][j] and lista[2][i] <= listc[1][j] and lista[0][i] == listc[0][j] and lista[8][i] == 'intended value' :
                green += listc[2][i]
            if lista[7][i] >= listc[1][j] and lista[2][i] <= listc[1][j] and lista[0][i] == listc[0][j] and lista[8][i] != 'intended value' :
                green += listc[2][i]
            if lista[2][i] >= listc[1][j] and lista[3][i - 1] <= listc[1][j] and lista[0][i] == listc[0][j]:
                black += listc[2][i]
    toc = timeit.default_timer()
    if i % 100 == 0:
        print('processing algorithm: {}'.format(toc - tic))
        print('we are at row {}'.format(i))
    output[0].append(lista[1][i])
    output[1].append(red)
    output[2].append(green)
    output[3].append(black)
    red = 0
    green = 0
    black = 0
toc = timeit.default_timer()
np.savetxt("outfile" + str(x)
           + ".csv", np.column_stack((output[0], output[1], output[2], output[3])), delimiter=",", fmt='%s')
tac = timeit.default_timer()
print('exporting {}'.format(tac - toc))

For me, the problem starts with the nested loops showing no specification of what is to be achieved, not even a suggested abstraction (being the body of a function given a name).

Observations:

the output does seem to depend on the order of elements of lista
(even beyond its order: lista[3][i-1])
• hope lista[2][i] >= listc[1][j] is never True for i 0
 (unless you want lista[3][-1] accessed)
the output does not seem to depend on the order of elements of listc
both lista and listc are not changed
→ the "range conditions" won't change unless at least one index changes
all of "the increments" share the condition lista[0][i] == listc[0][j]
the conditions between lista[6/7][i] and listc[1][j] are not complementary for including equality in both cases

implying red/green possibly getting incremented twice in a single iteration (not using else)

idea:

document, in the code, what is to be achieved
Python supports this with docstrings
use telling names
have a tool help you sticking to The Python Style Guide
order listc
for each i, iterate only that part of the ordered listc where lista[0][i] == listc[0][j]
ignore if lista and listc are not "rectangular":

food for thought: untested result of refactoring (get tool support for such, too)
(here extracting local variables, mostly)

list_c = sorted(listc)
for i in range(len(lista[1])):
    red = green = black = 0
    a0i = lista[0][i]
    first = bisect_left(list_c[1], a0i)
    beyond = bisect_right(list_c[1], a0i, first)
    if first < beyond:
        a2i = lista[2][i]
        a3i = lista[3][i]
        c2i = list_c[2][i]
        a8i_intended = lista[8][i] == 'intended value'
        for j in range(first, beyond):
            c1j = list_c[1][j]
            if (c1j <= a3i or c1j >= a2i):
                if lista[7][i] >= c1j and lista[6][i] <= c1j and a8i_intended:
                    red += c2i
                if a3i >= c1j and lista[7][i] <= c1j and not a8i_intended:
                    red += c2i
                if lista[6][i] >= c1j and a2i <= c1j and a8i_intended:
                    green += c2i
                if lista[7][i] >= c1j and a2i <= c1j and not a8i_intended:
                    green += c2i
                if a2i >= c1j and lista[3][i - 1] <= c1j:
                    black += c2i
    toc = timeit.default_timer()
    if i % 100 == 0:
        print('processing algorithm: {}'.format(toc - tic))
        print('we are at row {}'.format(i))
    output[0].append(lista[1][i])
    output[1].append(red)
    output[2].append(green)
    output[3].append(black)

afterthought:  it may be better to handle listc[1][j] <= lista[3][i] and lista[2][i] <= listc[1][j] separately


Testing quicksort user input I am testing a quicksort implementation. Arrays of random size with random data are passed to quicksort and the return time is averaged out. The user is prompted to enter values for

minimum length of array
maximum length of array
maximum value in array
minimum value in array
if the user wishes to see the sorted and unsorted arrays
if the user wishes the program to check itself by testing to make sure the array was sorted properly

I tried breaking up the prompts for user input into several methods but things are getting ugly. Are there any guidelines to follow for when prompting the user for information? One thing I'm not sure about is when to prompt with a question mark vs a colon (e.g. "what is x?" vs "input x:").

public static void main(String[] args)
{
    getInput();
}

public static void getInput()
{
    int bound = getNumber("How many arrays should be generated for quicksort?");
    int minVal = getNumber("Minimum value of element:");
    int maxVal = getNumber("Maximum value of element:");
    int minLength = getNumber("Minimum length of an array:");
    int maxLength = getNumber("Maximum length of an array:");
    boolean giveOutput = yes_no("Would you like to see the sorted and unsorted arrays?");
    boolean checkCorrectness = yes_no("Check proccessed arrays to see if sorted correctly?");

    long totalTime = 0, sumArraySizes = 0;

    for(int i = 0; i < bound; i++)
    {
        int[] sample = generateRandomArray(minLength, maxLength, minVal, maxVal);
        sumArraySizes += sample.length;
        if(giveOutput)
            System.out.println(Arrays.toString(sample));
        long startTime = System.nanoTime();//was using System.currentTimeMillis(); but endTime and startTime would always be the same
        qs(sample, 0, sample.length-1);
        long endTime = System.nanoTime();
        totalTime += endTime-startTime;
        if(giveOutput)
            System.out.println(Arrays.toString(sample)+"\n");
        if(checkCorrectness)
            if(!isSorted(sample))
                System.out.println("WARNING! mistake detected");
    }

    System.out.println("Average size of array: "+sumArraySizes/bound);
    System.out.println("Average time taken to sort array: "+totalTime/bound+" nanoseconds");
}

/*prompts user with a message and returns number they enter*/
public static int getNumber(String message)
{
    Scanner sc = new Scanner(System.in);
    System.out.println(message);
    return sc.nextInt();
}

/*http://stackoverflow.com/questions/15927885/yes-no-with-boolean-or-if-else*/
public static boolean yes_no(String message)
{
    Scanner sc = new Scanner(System.in);
    String answer;
    boolean yn;

    System.out.print(message);
    OUTER:
    while (true) {
        answer = sc.nextLine().trim().toLowerCase();
        switch (answer) {
            case "y":
                yn = true;
                break OUTER;
            case "n":
                yn = false;
                break OUTER;
            default:
                System.out.println("Please answer with y or n");
                break;
        }
    }
    return yn;
}

What should the method yes_no be named? I wanted to call it yesOrNo but that looked strange.

Spaces

Put a space after for and if:

for(int i = 0; i < bound; i++) --> for (int i = 0; i < bound; i++)
if(giveOutput) --> if (giveOutput)

Put a space around a -:

sample.length-1 --> sample.length - 1

And the same goes for +:

System.out.println("Average time taken to sort array: "+totalTime/bound+" nanoseconds");

becomes:

System.out.println("Average time taken to sort array: " + totalTime/bound + " nanoseconds");

if-statements

Add braces around if-statements:

if (giveOutput) {
    System.out.println(Arrays.toString(sample));
}

Avoid nested if-statements such as:

if(checkCorrectness)
    if(!isSorted(sample))
        System.out.println("WARNING! mistake detected");

Change that to:

if (checkCorrectness && !isSorted(sample)) {
    System.out.println("WARNING! mistake detected");
}

(Consider throwing an exception for this by the way. A mistake is important stuff)

Naming

Whatever qs is doing (A quicksort I guess), the name is too short to be meaningful. Name it quicksort instead.

Name your methods for inputting things as input*:

getNumber --> inputNumber
yes_no --> inputBoolean

  Are there any guidelines to follow for when prompting the user for information? One thing I'm not sure about is when to prompt with a question mark vs a colon (e.g. "what is x?" vs "input x:").

I believe this can be answered better on User Experience. They probably have a question about it already.

I would think that a question mark should be used when asking a question, and a colon should be used when encouraging the user to enter something. Of course this doesn't say much though as all your user prompts can be modified to the other form. My opinion is that what you currently have is good though.

Other comments

Good choice to use System.nanoTime();
To make the existence of another line in your output clear, I would change System.out.println(Arrays.toString(sample)+"\n"); to:

if (giveOutput) {
    System.out.println(Arrays.toString(sample));
    System.out.println();
}

In Java, the convention is that { goes on the same line, not on it's own line.

public static int getNumber(String message)
{

should be, according to the conventions:

public static int getNumber(String message) {

Avoid multiple variable declarations on the same line

long totalTime = 0, sumArraySizes = 0;

can be:

long totalTime = 0;
long sumArraySizes = 0;


Logging observable values while extending service I'm trying to extend a service function which returns an Observable<T>. The idea is to extend the function so I can add logging functionality whenever the observable emits a new value.

fromEvent<T>(eventName: string): Observable<T> {
    super.fromEvent(eventName).subscribe(
        function next(data) {
            console.group();
                console.log('----- SOCKET INBOUND -----');
                console.log('Action: ', eventName);
                console.log('Payload: ', data);
            console.groupEnd();
        }
    );

    return super.fromEvent(eventName);

How can I make my function more effective? Because whenever fromEvent() is invoked a subscription is created and an observable is returned from the base class. As I understand, all subscriptions should at some point be unsubscribed.

Or are there any other way you could implement a logging functionality on an observable that listens to a specific event?

One way to achieve what you want is to use .do() and may be .catch() operators.

.do() operator taps on a target stream and executes a function on each non-error event in the stream. It is designed specifically for this purpose, which you can define more generically as side effects:

  This method can be used for debugging, logging, etc. of query behavior by intercepting the message stream to run arbitrary actions for messages on the pipeline. -- RxJs docs

Notice that .do():

does NOT affect the original Observable stream in any way. This function itself returns an Observable of the same type as the original one.
does NOT subscribe to the original Observable, so the .subscribe() invocation is required down the road.
does NOT react to error events in the original Observable. Therefore, you may want to use .catch() to log the errors.

fromEvent<T>(eventName: string): Observable<T> {
  return super
    .fromEvent(eventName)
    .do(eventData => {
      console.group();
      console.log('----- SOCKET INBOUND -----');
      console.log('Action: ', eventName);
      console.log('Payload: ', eventData);
      console.groupEnd();
    })
    .catch(error => {
      console.group();
      console.error('----- SOCKET INBOUND -----');
      console.error('Action: ', eventName);
      console.error('Error: ', error);
      console.groupEnd();
    });
}


Very simple events implementation I've been thinking about implementing events and wrote some abstract code:

#include <memory>
#include <vector>
#include <iostream>

template<typename Event>
class Dispatcher
{
public:
    class Listener
    {
    public:
        virtual void OnEvent(Event& event, Dispatcher& sender) {};
    };

private:
    typedef std::shared_ptr<Listener> ListenerPtr;
    typedef std::vector<ListenerPtr> Listeners;
    Listeners _listeners;

public:
    void Reg(ListenerPtr listener)
    {
        if( _listeners.end() != std::find(_listeners.begin(), _listeners.end(), listener) )
            return;

        _listeners.push_back(listener);
    }

    void Unreg(ListenerPtr listener)
    {
        Listeners::iterator iter = std::find(_listeners.begin(), _listeners.end(), listener);
        if( _listeners.end() == iter )
            return;

        _listeners.erase(iter);
    }

    void Dispatch(Event& event)
    {
        for( Listeners::iterator iter = _listeners.begin(); iter != _listeners.end(); ++iter )
            (*iter)->OnEvent(event, *this);
    }
};

struct SomeEvent
{
    int someParam;
    SomeEvent(int someParam) : someParam(someParam) {}
};

class SomeDispatcher : public Dispatcher<SomeEvent>
{
};

struct OtherEvent
{
    int otherParam;
    OtherEvent(int otherParam) : otherParam(otherParam) {}
};

class OtherDispatcher :
    public SomeDispatcher, public Dispatcher<OtherEvent>
{
};

class Consumer :
    public Dispatcher<SomeEvent>::Listener,
    public Dispatcher<OtherEvent>::Listener
{
    virtual void OnEvent(SomeEvent& event, Dispatcher<SomeEvent>& sender)
    {
        std::cout << "OnEvent SomeEvent " << event.someParam << std::endl;
    }

    virtual void OnEvent(OtherEvent& event, Dispatcher<OtherEvent>& sender)
    {
        std::cout << "OnEvent OtherEvent " << event.otherParam << std::endl;
    }
};

int main(int argc, char **argv)
{
    OtherDispatcher dispatcher;

    std::shared_ptr<Consumer> consumer(new Consumer());

    dispatcher.Dispatcher<SomeEvent>::Reg( consumer );
    dispatcher.Dispatcher<OtherEvent>::Reg( consumer );

    dispatcher.Dispatcher<SomeEvent>::Dispatch(SomeEvent(1));
    dispatcher.Dispatcher<OtherEvent>::Dispatch(OtherEvent(2));

    dispatcher.Dispatcher<SomeEvent>::Unreg( consumer );

    dispatcher.Dispatcher<SomeEvent>::Dispatch(SomeEvent(3));
    dispatcher.Dispatcher<OtherEvent>::Dispatch(OtherEvent(4));

    return 0;
}

It compiles and works as expected, but... I have to write more code to inherit from Dispatcher and Listener, to register listener and to define more exactly which dispatcher I'm registering to. And this method does not allow registering of custom class method, but only OnEvent().

Apart from this, are there any issues with this code? Is it compatible with different compilers and platforms? Any suggestions?

Listner needs a virtual destructor
and the OnEvent() method should probably be pure virtual:

class Listener
{
public:
    virtual void OnEvent(Event& event, Dispatcher& sender) {};
};

// Try

class Listener
{
    public:
        virtual ~Listener() {}
        virtual void OnEvent(Event& event, Dispatcher& sender) = 0;
};

You need the virtual destructor because delete will more than likely be called via a pointer to the base class (Listener) at some point.

No point in implementing OnEvent() to do nothing if you need sombody to over-ride it.

Personally I would prefer to use boost::ptr_vector<> rather than a std::vector<std::shared_ptr<>>. The boost vector has a couple of advantages (less lcoks required). Also its members are provided via references rather than pointers so it makes it more natural to use with the standard algorithms (not such a big deal in C++11 (with lambdas) but it makes C++03 much easier to use).

typedef std::shared_ptr<Listener> ListenerPtr;
typedef std::vector<ListenerPtr> Listeners;
Listeners _listeners;

// Replace with:

boost::ptr_vector<Listener>  listeners;

Be careful of using '_` on the front of identifiers. The rules are not obvious so best avoided to make sure you don't actually break them. Underscore Rules

Personally not a fan of the following:

   if( _listeners.end() != std::find(_listeners.begin(), _listeners.end(), listener) )

Nothing wrong with it. But reversing the order of the test just makes you pause a second. See Yoda Conditionals.

Prefer to use standard algorithms rather than hand coded loops.

   for( Listeners::iterator iter = _listeners.begin(); iter != _listeners.end(); ++iter )
        (*iter)->OnEvent(event, *this);

   // Try
   std::for_each(listeners.begin(), listeners.end(),
                 std::bind2nd(Listner::OnEvent, event)
                ); // Assume boost::ptr_vector<>

   // Or go all C++11 on us
   std::for_each(std::begin(listeners), std::end(listeners),
                 [&event](std::shared_ptr<Listener>& lis) {lis->OnEvent(event);}
                );

If you want to have listeners that only deal with a specific type of event. You can write your Listner to be a filtering lsitener.

class Listener
{
public:
    virtual void OnEvent(Event& event, Dispatcher& sender) {};
};

Change to:

template<class ForEvent>
class Listener
{
    public:
        ~Listener() {}

        // This is the method called by the dispatcher.
        void eventFilter(OnEvent& event, Dispatcher& sender)
        {
             // dynamic_cast to a pointer type.
             // Result is NULL if not the correct type.
             // If you used a value type then it would throw an exception.
             ForEvent* specialEvent = dynamic_cast<ForEvent*>(&event);
             if (specialEvent != NULL)
             {
                 // Only call the OnEvent for the types we want.
                 // put back to a references (so OnEvent does not need to
                 // check for NULL).
                 this->OnEvent(*specialEvent, sender);
             }
        }

        virtual void OnEvent(ForEvent& event, Dispatcher& sender) = 0;
};


Splitting the Java builder to avoid writing same code multiple times I have a piece of code which looks like below:

Problem hashProblem;
String indexName;
if(condition.getOwner() != null) {
    indexName = sourceLocation ;
    hashProblem = new Problem().builder()
        .locationID(condition.getLocationID())
        .sourceLocation(condition.getSourceLocation())
        .build();
}
else {
    indexName = currentLocation;
    hashProblem = new Problem().builder()
        .locationID(condition.getLocationID())
        .currentLocation(criteria.getcurrentLocation())
        .build();
}

Is there a way to write this code in a more elegant way? While building the hashProblem object setting the locationID is always required. I am unable to think of a way to split the builder so that I could write .locationID only once.

The builder instance can be extracted into a dedicated reference and you'll be able to apply the common instructions on it only once, for example:

final Problem.Builder builder = new Problem().builder().locationID(condition.getLocationID());
final String indexName;
if (condition.getOwner() != null) {
  indexName = sourceLocation;
  builder.sourceLocation(condition.getSourceLocation());
} else {
  indexName = currentLocation;
  builder.currentLocation(criteria.getcurrentLocation());
}
final Problem hashProblem = builder.build();


Using a dictionary template for consistent response structure from an API I am making a simple API for news items using Python and the Bottle framework.

I return a Python dictionary from my endpoints as Bottle converts this to JSON automatically in the response to the client.

I wanted to ensure a consistent structure to the responses, so I have declared a template dictionary. The code for each endpoint makes a deep copy of this then modifies the relevant data before returning it.

import bottle
from bottle import route
from copy import deepcopy as deepcopy

# Dictionary for response template
response_dict = {
"status" : "ok",
"code" : 0,
"error" : False,
"message" : "",
"result" : {}
}

# Example route
@route('/example')
def example():
    return_dict = deepcopy(response_dict)
    return_dict["message"] = "Success"
    return_dict["result"] = {
        "title" : "Test Title",
        "body" : "
Lorem ipsum dolor sit amet...
"
    }
    return return_dict

I would like to know if there is a better way to use a template for a JSON response and whether the structure of my dictionary is appropriate.

There is no need to use deepcopy here. Just have a function that returns a new object when you need it. And while you're at it, just make the message and the result parameters of that function. Or even all of them:

def json_response(message, result, status="ok", code=0, error=False):
    return {
        "status" : status,
        "code" : code,
        "error" : error,
        "message" : message,
        "result" : result
    }

@route('/example')
def example():
    result = {
        "title" : "Test Title",
        "body" : "
Lorem ipsum dolor sit amet...
"
    }
    return json_response("Success", result)

Having all as parameters, but with default values, allows you to do this in the future:

json_response("Failure", None, status="fail", code=404, error=True)

In the end it depends on how often you need to use this template whether or not this is better than just directly returning the dictionary explicitly:

@route('/example')
def example():
    return {
        "status" : "ok",
        "code" : 0,
        "error" : False,
        "message" : "Success",
        "result" : {
            "title" : "Test Title",
            "body" : "
Lorem ipsum dolor sit amet...
"
        }
    }


Advent of Code 2017 Day 1 Using Advent of Code as a source of toy problems for my first dabblings in Rust. Looking for feedback from more-seasoned Rustaceans on usage of the language.

Advent of Code Day 1: Inverse Captcha

use std::fs::File;
use std::io::prelude::*;

fn get_input() -> Vec<u8> {
    let mut file = File::open("input.txt").unwrap();
    let mut content = Vec::new();

    file.read_to_end(&mut content).unwrap();
    for i in 0 .. content.len() {
        content[i] = content[i] - ('0' as u8);
    }

    content
}

fn offset_sum(input: &[u8], offset: usize) -> i32 {
    let len = input.len();
    let mut sum = 0;

    for i in 0 .. len {
        let j = (i + offset) % len;
        if input[i] == input[j] {
            sum += input[i] as i32;
        }
    }

    sum
}

fn main() {
    let input = get_input();

    println!("Part 1 total: {}", offset_sum(&input, 1));
    println!("Part 2 total: {}", offset_sum(&input, input.len()/2));
}

To be honest, for a simple, one-off problem, I disagree that hardcoding an input file name is disadvantageous. Similarly for error handling; you know the file is there and will read fine, so unwrapping it is fine; if it fails for some stupid reason there's no recovery you can do anyway.

And because the problem is so simple, there's not much to go about complaining about. This is a perfectly fine solution, because it's very readable.

But there are a few places where the standard library could be better utilized: (Note: I may go a little far here. Go as far iterator as you feel comfortable.)

The iteration over content's indices in get_input can be replaced by a mutating iterator:

for byte in &mut content {
    *byte -= b'0';
}

Here I've also used a byte character literal, rather than casting to u8, which clippy will warn you about. With a known constant, it's safe, but then you should just use the byte literal.

What this says is to, for each position in content, take the thing at the position *byte, and decrement (-=) it by b'0'. It's the exact same as what you did, except without the manual indexing. (It's probably lowered to the same machine code in the end, but iterators are fun!)

offset_sum provides a more interesting field for refactoring, though. It can be done in one iterator chain by cleverly utilizing the available tools.

input

Start with the slice,

    .iter()

and get an iterator from it.

    .cloned()

Since we're operating over a Copy type, I dereference it here for convenience. If we just call iter, we get an iterator over &u8, but calling cloned clones each element, which in this case is just a cheap Copy. Don't go doing this with non-copy types, though; this is just a convenience.

    .enumerate()

We pair up each byte with its index in the iterator.

    .filter(|&(i, b)| b == input[(i + offset) % input.len()])

Filter out all of the numbers that don't match our requirements.

    .map(|(_, b)| u32::from(b))

Map our iterator back to just the number we care about. Convert it to a u32 so that we can

    .sum()

it into a u32 and consume the iterator.

Try it Online!

Some fun extensions you might try:

This is always going to be an \$O(n)\$ algorithm because you have to scan the whole string. But rather than handle indices, in the first half you could use  windows to grab each neighboring pair of bytes, though you will have to handle wraparound somehow. In the second half, you can reduce the number of comparisons you have to do by half, because each number is compared against the same number in either direction (the absolute opposite of a circular buffer). Maybe you can split the slice into two halves, then zip the two halves together for comparisons?


Iterating through nested array of objects I have the following JSON list of objects, and I'm able to find the required object, and pass it back to the caller, given the id. Everything works fine, I'm just wondering if there's a better (more efficient) way to return the node. I'm open to using a 3rd party tool like lodash.

JSON payload:

{
  "_expanded": true,
  "_canDrop": false,
  "_id": "-1",
  "_name": "root",
  "_children": [
    {
      "_expanded": true,
      "_canDrop": false,
      "_id": "1",
      "_name": "Child 1",
      "_children": [
        {
          "_expanded": true,
          "_canDrop": false,
          "_id": "1-1",
          "_name": "Child 1-1",
          "_children": [
            {
              "_expanded": false,
              "_canDrop": false,
              "_id": "1-1-1",
              "_name": "Child 1-1-1",
              "_children": []
            }
          ]
        },
        {
          "_expanded": false,
          "_canDrop": false,
          "_id": "1-2",
          "_name": "Child 1-2",
          "_children": []
        },
        {
          "_expanded": false,
          "_canDrop": false,
          "_id": "1-3",
          "_name": "Child 1-3",
          "_children": []
        }
      ]
    },
    {
      "_expanded": true,
      "_canDrop": false,
      "_id": "2",
      "_name": "Child 2",
      "_children": [
        {
          "_expanded": false,
          "_canDrop": false,
          "_id": "2-2",
          "_name": "Child 2-2",
          "_children": []
        }
      ]
    }
  ]
}

Find method:

public findNode = (id: any): TreeNode => {
    let result = null;
    if (this._id === id) {
      result = this;
    } else {
      if (this._children.length > 0) {
        for (let index = 0; index <= this._children.length - 1; index++) {
          result = this._children[index].findNode(id);
          if (result) {
            break;
          }
        }
      }
    }
    return result;
  }

Your method already looks good! I would only make a couple small changes to the logic as follows:

The if (this._children.length > 0) isn't necessary as the for loop
will make the check for you
Instead of using a standard for loop, I would prefer a for..of
loop. Typescript will transpile this so it will work in any
environment
I prefer early returns to nested statements. Because of this, I
would recommend storing the result variable in as small a block as
possible

With these changes:

public findNode = (id: any): TreeNode => {
  if (this._id === id) {
    return this;
  }

  for (const child of this._children) {
    const result = child.findNode(id);
    if (result) {
      return result;
    }
  }

  return null;
}

Also, here's a couple Typescript specific notes:

Avoid any like the plague. When you use any you are telling
typescript to effectively ignore any mistakes in your code. Given
your data structure, it looks like id should be of type string
I highly recommend turning on
strictNullChecks.
This will help prevent a large class of errors
Methods are public by default in Typescript, with this in mind I
recommend dropping the public modifier in most cases as it is redundant. However, this does not apply if you are working on a team with developers who generally work with other languages as it could lead to confusion in the team. For example, in C# the default is
that all methods are private

With these changes:

findNode = (id: string): TreeNode | null => {
  if (this._id === id) {
    return this;
  }

  for (const child of this._children) {
    const result = child.findNode(id);
    if (result) {
      return result;
    }
  }

  return null;
}


Django how to make this view faster? I have view and it works correct, but very slow

class Reading(models.Model):
    meter = models.ForeignKey(Meter, verbose_name=_('meter'))
    reading = models.FloatField(verbose_name=_('reading'))
    code = models.ForeignKey(ReadingCode, verbose_name=_('code'))
    date = models.DateTimeField(verbose_name=_('date'))

    class Meta:
        get_latest_by = 'date'
        ordering = ['-date', ]

    def __unicode__(self):
        return u'%s' % (self.date,)

    @property
    def consumption(self):
        try:
            end = self.get_next_by_date(code=self.code, meter=self.meter)
            return (end.reading - self.reading) / (end.date - self.date).days
        except:
            return 0.0

    @property
    def middle_consumption(self):
        data = []
        current_year = self.date.year
        for year in range(current_year - 3, current_year):
            date = datetime.date(year, self.date.month, self.date.day)
            try:
                data.append(Reading.objects.get(
                    date = date,
                    meter = self.meter,
                    code = self.code
                ).consumption)
            except:
                data.append(0.0)
            for i in data:
                if not i:
                    data.pop(0)
        return sum(data) / len(data)

class DataForDayChart(TemplateView):
    def get(self, request, *args, **kwargs):
        output = []
        meter = Meter.objects.get(slug=kwargs['slug'])
        # TODO: Make it faster
        for reading in meter.readings_for_period().order_by('date'):
            output.append({
                "label": reading.date.strftime("%d.%m.%Y"),
                "reading": reading.reading,
                "value": reading.consumption / 1000,
                "middle": reading.middle_consumption / 1000
            })
        return HttpResponse(output, mimetype='application/json')

What should I change to make it faster?

There's nothing intrinsically time-consuming here code-wise - I think the best approach is to make sure that your database tables are set up correctly with the required indices. Perhaps create a view on the db to push some work onto that rather than select in code?

I am a little puzzled however by your middle_consumption function. The inner loop contents do something like:

get a date, x years ago from present day
get a reading on that date, or 0 on failure
add that reading to the results
go through the results
    if the current item is 0, delete the **first** item

This seems wrong.

What happens on February 29th? It would be better to add 365 days if that's appropriate for your application.
Why add a value only to (presumably want to) delete it again?  Would something like this be better?

try:
    current_value = Reading.objects.get(
        date = date_of_reading,
        meter = self.meter,
        code = self.code
    ).consumption
except Reading.DoesNotExist: # or suitable exception
    current_value = 0
if current_value > 0:
    anniversary_values.append( current_value )


Computer tries to guess your inputted number I'm rehashing some of the basics of c++ and I just want to find useful points along the way of things I can do to improve my coding.
This program has the user enter a number, then the computer try to guess it, with some higher/lower logic.
#include <iostream>
#include <cstdlib>
#include <ctime>

int main()
{
    srand(static_cast<unsigned int>(time(0)));  //seed random number generator

    int secretNumber = 0;
    int tries = 0;
    int guess;

    int highRange = 100;
    int lowRange = 0;

    std::cout << "\tWelcome to Guess My Number\n\n";

    std::cout << "\tPlease enter your number between 1 and 100:\n";

    std::cin >> secretNumber;

    do
    {
        do
            guess = rand() % highRange + 1;
        while (guess <= lowRange);

        ++tries;

        if (guess > secretNumber)
        {
            std::cout << "Computer guess was too high! (" << guess << ")\n\n";
            highRange = guess;
        }
        else if (guess < secretNumber)
        {
            std::cout << "Computer guess was too low! (" << guess << ")\n\n";
            lowRange = guess;
        }
        else
        {
            std::cout << "\nI guessed it! (" << guess << ") in " << tries << " guesses!\n";
        }

    } while (guess != secretNumber);

    return 0;
}

You can simplify this:
    do
        guess = rand() % highRange + 1;
    while (guess <= lowRange);

Why not get a value in the correct range then add lowRange.
    // I did not check the maths there
    // may be an off by one error in this
    // Please verify the range is correct.
    guess = rand() % (highRange + 1 - lowRange);
    guess += lowRange;

I am not complain about you using rand() in this extremely simple example code. But you should note that:

rand() is not very random.
rand() % x not all values in the range [0-x) are equally probably.

Please have a look at the C++ standard random number generator (you are using the C version).
http://www.cplusplus.com/reference/random/

You use the correct C++ header files:
#include <cstdlib>
#include <ctime>

This puts all the functionality in the std namespace. Though it allows implementations to also add a definition in the global namespace (though this is optional). But this optional bit is what allows it to compile on your system (but you can't assume it would work everywhere).
So the functions you use should be prefixed with std::.
std::srand(static_cast<unsigned int>(std::time(0)));
^^^^^                                ^^^^^

....
std::rand() % highRange
^^^^^


Create a binary search tree from a sorted array (increasing order) Description:

Since the input array is sorted, we can take the middle element of the array to be parent and recursively do the same on the left and right halves to create left and right sub trees of the parent.

class Node{
        int data;
        Node left;
        Node right;
        Node(int x)
        {
            data = x;
        }
    }
    public class createBSTFromSortedArray {

        public static Node getBST(int[] sortedArray, int start, int end)
        {
            if(start <= end)
            {
            int mid = (start+end)/2;
            Node root = new Node(sortedArray[mid]);
            root.left = getBST(sortedArray,start,mid-1);
            root.right = getBST(sortedArray, mid+1,end);
            return root;
            }
            return null;
        }

        public static void printTree(Node root)
        {
            if(root == null)
            {
                return ;
            }

            printTree(root.left);
            System.out.println(root.data);
            printTree(root.right);
        }
        public static void main(String[] args) {
            int[] input = {};
            Node root = getBST(input, 0 , input.length - 1);
            printTree(root);
            }
    }

Space and indentation

Your indentation and use of spaces is terrible. This document may help you improve your Java coding style.

Possible overflow

You solution may cause overflow and unexpected results for big enough inputs in this line:

int mid = (start + end) / 2;

changing it to this will solve the problem:

int mid = start + (end - start) / 2;

Also returning null early looks cleaner IMO

public static Node getBST(int[] sortedArray, int start, int end) {
    if(start > end) {
        return null;
    }
    int mid = start + (end - start) / 2;
    Node root = new Node(sortedArray[mid]);
    root.left = getBST(sortedArray, start, mid - 1);
    root.right = getBST(sortedArray, mid + 1, end);
    return root;
}