A review of my jQuery Slider I have created a jQuery slider for my personal website and I wanted to get some feedback on how it looks and if there are things I can do to make it better.

You can see the live demo here: http://www.nickmeagher.com

And here is the code:

        var sliderWidth = 0,
        marginLeft = 0,
        containerWidth = 1040,
        slideSpeed = 600,
        slideDuration = 6000,
        autoSlide = 0; // (0) = off (1) = on

function slide_right() {
        if ( (sliderWidth - Math.abs(marginLeft)) > containerWidth ) {
           $('.portfolio-inner-wrapper').stop().animate({
            'margin-left': '+=-' + containerWidth
           }, slideSpeed, function() {
            marginLeft += -containerWidth;
        });
     } else {
        $('.portfolio-inner-wrapper').stop().animate({
            'margin-left': '0px'
        }, slideSpeed, function() {
            marginLeft = 0;
        });
    }
}

function slide_left() {
    if ( (sliderWidth - Math.abs(marginLeft)) < containerWidth ) {
            $('.portfolio-inner-wrapper').animate({
                'margin-left': '+=+' + containerWidth
            }, slideSpeed, function() {
                marginLeft += containerWidth;
            });
    } else {
        $('.portfolio-inner-wrapper').animate({
                'margin-left': '0'
            }, slideSpeed, function() {
                marginLeft = 0;
            });
    }
}

// Iterate through each container to calculate width
$('.work-container').each(function() {
    sliderWidth += $(this).outerWidth( true );
});

// Set width of inner wrapper
$('.portfolio-inner-wrapper').css('width', sliderWidth);

$('a.next').click(function() {
    slide_right();
});

$('a.prev').click(function() {
    slide_left();
});

if (autoSlide == 1) {
    setInterval(function() {
        slide_right();
    }, slideDuration);
}

Here are some general things you should be doing differently. These suggestions don't change the functionality of your slider, and are suggestions you can apply to your future projects.

First and most important

var wrapper = $('.portfolio-inner-wrapper');
//As a rule of thumb, if you use a selector more than once, you should cache it.
    //Now use 'wrapper' everywhere you used $('portfilo...') instead.
    //Cache it outside if you want to use it in other functions as well.
//Ex:
wrapper.css('width', sliderWidth);

Next:

if (autoSlide) { //Don't need == 1. Zero is a falsy value.
    setInterval(function() {
        slide_right();
    }, slideDuration); //You don't need setInterval either. jQuery has a built in queue.
}

When you have multiple 'animate' calls, jQuery puts them in a queue to execute them one by one. This goes for the rest of your slider as well.

Save function calls. Do it like this:

$('a.next').on('click', function() {
        if ( (sliderWidth - Math.abs(marginLeft)) > containerWidth ) {
           wrapper.stop().animate({
            'margin-left': '+=-' + containerWidth
           }, slideSpeed, function() {
            marginLeft += -containerWidth;
        });
     } else {
        wrapper.stop().animate({
            'margin-left': '0' //Zero doesn't need a unit of measure (ie. px, em, %, etc)
        }, slideSpeed, function() {
            marginLeft = 0;
        });
    }
});

When you use the .click() method, jQuery calls the .on() method. Then if you still call another function inside that, that's three function calls that could be spared if you simply used this:

$('a').on('click', function() {
    do.stuff
});

Finally, I'll be happy to answer any other questions you might have.


Builder pattern that forces all members to be initialized at compile-time This java answer made me wonder whether it would be possible to implement the builder pattern in C++ in a way that checks at compile-time whether all necessary members of the constructed object are actually set before the object is constructed. This is what I came up with:

struct Foo {
    int x_;
    int y_;
    int z_;
};

template <bool x_set = 0, bool y_set = 0, bool z_set = 0>
struct FooBuilder {
    int x_, y_, z_;

    Foo build() {
        static_assert(x_set && y_set && z_set, "all members must be initialized before building a Foo!");
        return { x_, y_, z_};
    }

    FooBuilder<1,y_set,z_set> x(int val) {
        FooBuilder<1,y_set,z_set> result;
        result.x_ = val;
        result.y_ = y_;
        result.z_ = z_;
        return result;
    }

    FooBuilder<x_set,1,z_set> y(int val) {
        FooBuilder<x_set,1,z_set> result;
        result.x_ = val;
        result.y_ = y_;
        result.z_ = z_;
        return result;
    }

    FooBuilder<x_set,y_set,1> z(int val) {
        FooBuilder<x_set,y_set,1> result;
        // optional check to allow z to be set only once
        static_assert(!z_set, "z may not be initialized twice!");
        result.x_ = val;
        result.y_ = y_;
        result.z_ = z_;
        return result;
    }
};

int main() {
    FooBuilder<> b;
    auto f1 = b.x(1).y(2).z(3).build();
    auto f1a = b.z(3).y(2).x(1).build();
    auto f2 = b.x(1).y(2).build(); // fails with static assertion
    auto f3 = b.x(1).y(2).z(3).z(4).build(); // fails with static assertion
    auto partially_initialized_builder = b.x(1);
    auto f4 = partially_initialized_builder.build(); // fails...
    auto f5 = partially_initialized_builder.y(4).z(6).build(); // runs

    return 0;
}

With this approach, I like the fact that programming errors that result from forgotten arguments to the builder can be caught at compile-time. However, I'm not sure if it is too verbose, since the whole content of the builder has to be copied/moved in each setter method (even if the copies might be optimized away).

So my main questions are:

Would such a pattern actually be useful in practice?
Is there an elegant way of sharing the member variables of the Builder objects, despite the results of each setter call being of a different type? (For cheap-to-move types, I am thinking along the lines of a "data block"-like struct, that gets moved from this into the result.)
Any other downsides to this approach?

P.S.: Please disregard the lack of access control (x_ etc. should be private), this question is more about the high-level picture ;-)

Would such a pattern actually be useful in practice?

Not really in my opinion. In C++20 you can use designated initializer lists like {.min = 10, .max = 20} and I mean if the argument is not clear from the context than there are various ways to improve this, even today.

A comment. Foo f(/*min=*/10);
User-defined literals Rectangle r(10_width, 200_height);
Strong types: Player p(Position(10, 10));
Variables: const int size = 10; Square s(size);

This also doesn't work with non-copyable, non-moveable types, although those are rare in practice. The members also need to be default constructible unfortunately.

Some thoughts:

Note that your code has undefined behavior, since two of the variables are not set on the first call.
The build function is required in Java, but in C++ you can use a conversion operator. Although this suffers from the same problem that expression templates have: Using type deduction will not get you a Foo.
Delete copy construction for the builder object, since there is no reason to copy it.
Don't use 0 and 1 for bools please; false and true is clearer IMO.
Consider using noexcept and constexpr (if applicable).

  Is there an elegant way of sharing the member variables of the Builder objects, despite the results of each setter call being of a different type? (For cheap-to-move types, I am thinking along the lines of a "data block"-like struct, that gets moved from this into the result.)

Yes that's a good idea. Here's my take on it:

template <bool x_set = false, bool y_set = false, bool z_set = false>
struct FooBuilder {
  constexpr FooBuilder() noexcept = default;
  FooBuilder(const FooBuilder &) = delete;
  FooBuilder &operator=(const FooBuilder &) = delete;

  constexpr auto x(int val) noexcept {
    members_.x_ = val;
    return FooBuilder<true, y_set, z_set>(std::move(*this));
  }
  constexpr auto y(int val) noexcept {
    members_.y_ = val;
    return FooBuilder<x_set, true, z_set>(std::move(*this));
  }
  constexpr auto z(int val) noexcept {
    static_assert(!z_set, "cannot set z twice!");
    members_.z_ = val;
    return FooBuilder<x_set, y_set, true>(std::move(*this));
  }

  constexpr operator Foo() noexcept {
    static_assert(x_set && y_set && z_set, "all members must be set");
    return {members_.x_, members_.y_, members_.z_};
  }
  constexpr Foo build() noexcept { return *this; }

private:
  template <bool... args>
  constexpr FooBuilder(FooBuilder<args...> &&other) noexcept
      : members_{std::move(other.members_.x_), std::move(other.members_.y_),
                 std::move(other.members_.z_)} {}

  template <bool, bool, bool> friend struct FooBuilder;

  struct Proxy {
    int x_, y_, z_;
  } members_{};
};

There is still a lot of boilerplate, but unfortunately it cannot be removed since C++ doesn't have reflection yet.


Simplifying exception handling on Enumerators I have a files search function that iterates over a given directory until a given depth is reached and returns the found files. I did this via the Enumerate methods of the Directory class and yield return the results. Since it's very likely that I hit a directory that I can't access (e.g. system directories) or generate a too long path (if the depth is large), I have to catch the exceptions from these cases. However since I can't use yield in try/catch statements, I find my code pretty bloated, because I have to seperate the critical method calls from the rest.

Is there a better/shorter way to do this, or are there best practices for that case?

private IEnumerable<string> SearchSubdirs(string currentDir, int currentDepth) {

    IEnumerable<string> exeFiles;
    try {
        exeFiles = Directory.EnumerateFiles(currentDir, "*.exe");
    }
    catch (UnauthorizedAccessException uae) {
        Debug.WriteLine(uae.Message);
        yield break;
    }
    catch (PathTooLongException ptle) {
        Debug.WriteLine(ptle.Message);
        yield break;
    }

    foreach (string currentFile in exeFiles) {

        // Ignore unistaller *.exe files
        if (currentFile.IndexOf("unins", 0, StringComparison.CurrentCultureIgnoreCase) == -1) {
            yield return currentFile;
        }
    }

    if (currentDepth < maxDepth) {
        IEnumerable<string> subDirectories;
        currentDepth++;
        try {
            subDirectories = Directory.EnumerateDirectories(currentDir);
        }
        catch (UnauthorizedAccessException uae) {
            Debug.WriteLine(uae.Message);
            yield break;
        }
        catch (PathTooLongException ptle) {
            Debug.WriteLine(ptle.Message);
            yield break;
        }

        foreach (string subDir in subDirectories) {
            foreach (string file in SearchSubdirs(subDir, currentDepth)) {
                yield return file;
            }
        }
    }
}

I would use Linq methods to simplify the code. Also I've extracted 2 methods to simplify the main method. And please rename GetFilesWeAreLookingFor to whatever your find appropriate :).

private static IEnumerable<string> GetFilesWeAreLookingFor(string currentDir)
{
    try
    {
        return Directory.EnumerateFiles(currentDir, "*.exe")
            .Where(fileName => fileName.IndexOf("unins", 0, StringComparison.CurrentCultureIgnoreCase) == -1);
    }
    catch (UnauthorizedAccessException uae)
    {
        Debug.WriteLine(uae.Message);
    }
    catch (PathTooLongException ptle)
    {
        Debug.WriteLine(ptle.Message);
    }
    return Enumerable.Empty<string>();
}

private IEnumerable<string> GetNestedFiles(string currentDir, int nestedDepth)
{
    try
    {
        return Directory.EnumerateDirectories(currentDir)
            .SelectMany(subDirectory => SearchSubdirs(subDirectory, nestedDepth));
    }
    catch (UnauthorizedAccessException uae)
    {
        Debug.WriteLine(uae.Message);
    }
    catch (PathTooLongException ptle)
    {
        Debug.WriteLine(ptle.Message);
    }
    return Enumerable.Empty<string>();
}

private IEnumerable<string> SearchSubdirs(string currentDir, int currentDepth)
{
    IEnumerable<string> filesWeAreLookingFor = GetFilesWeAreLookingFor(currentDir);

    if (currentDepth < _maxDepth)
        filesWeAreLookingFor = filesWeAreLookingFor.Concat(GetNestedFiles(currentDir, currentDepth + 1));

    return filesWeAreLookingFor;
}


Separating logic per class and being able to use dependency injection I use Laravel, but hopefully this doesn't matter. I have a user and user's state. Say, simple integer or a string. I want to run some logic depending on user state in my service class.

So, the first approach is pretty simple:

<?php

class UserStateManager
{
    public function __construct(UserRepository $userRepository)
    {
        $this->userRepository = $userRepository;
    }

    public function handle(UserRepository $userRepository)
    {
        $user = $this->userRepository->find(1);
        switch ($user->state) {
            case 'new':
                $this->handleUserNewState();
                break;
            case 'banned':
                $this->handleUserBannedState();
                break;
            // ...
        }
    }

    private function handleUserNewState()
    {
        // Do some logic
        // Here we have access to userRepository which was injected in UserStateManager constructor
    }

}

Pay attention that I injected UserRepository in the constructor.

So, using this approach I keep UserStateManager dependencies injected, without creating them manually.

But now I want to put every state handler to a separate class. Like, to have UserNewState class with handle method.

The problem is that in this case:

public function handle(UserRepository $userRepository)
    {
        $user = $this->userRepository->find(1);
        switch ($user->state) {
            case 'new':
                (new UserNewState)->handle();
                break;
            case 'banned':
                (new UserBannedState)->handle();
                break;
            // ...
        }
    }

if I create UserNewState object I cannot inject dependencies into this class since I manually create it using new operator. Passing lots of dependencies in the constructor (like new UserNewState($this->userRepository, ...)) is not an option, ugly solution.

So how to deal with this case? Because it's very common: on the one hand I can separate logic into several methods inside one class, on the other hand I cannot separate logic into many classes.

The problem is dependency injection and creating these class manually. And I reckon that DI is a good thing.

Of course I can create dependencies inside UserNewState class, directly in the constructor:

public function __constructor() {
    $this->userRepository = new UserRepository;
}

but this is not testable. Even though I don't test my apps, I just want to follow best practices.

Without knowing exactly what those classes are supposed to do, I assume it's supposed to work something like (new UserNewState( $this->userRepository ))->handle( $user );. That doesn't look that bad to me.

What other dependencies do you envision your separate State classes are going to need besides the repository?

But perhaps something like a UserStateHandlerFactory is something you could be looking for?

class UserStateHandlerFactory
{
  private $userRepository;

  public function __construct( UserRepository $userRepository ) {
    $this->userRepository = $userRepository;
  }

  public function create( User $user ) {
    switch ($user->state) {
        case 'new':
            return new UserNewState( $this->userRepository );
            break;
        case 'banned':
            return new UserBannedState( $this->userRepository );
            break;
        // ...
    }
  }
}

class UserStateManager
{
  private $userRepository;
  private $handlerFactory;

  public function __construct( UserRepository $userRepository, UserStateHandlerFactory $handlerFactory )
  {
    $this->userRepository = $userRepository;
    $this->handlerFactory = $handlerFactory;
  }

  public function handle()
  {
    $user = $this->userRepository->find(1);
    $this->handlerFactory->create( $user )
                         ->handle( $user );
  }
}

However, I don't know if that makes things much cleaner though. It just looks like another layer of indirection; hiding away, what ultimately has to be done anyway: pass dependencies into the handlers.


Inversion count algorithm I implemented the inversion count algorithm using the merge sort approach for this question.

The question begins with a number t = number of test cases and then for each test case you input size of array and then the array entries.

For each test case we have to find the number of inversions = number of shifts made by insertion sort algorithm.

I was surprised to see that only 2 test case passed out of 13 and rest all time out. Can somebody suggest a way I should go about speeding up my code?

Kindly ignore the OOP principles for the time being.

import java.io.*;
import java.util.*;
import java.text.*;
import java.math.*;
import java.util.regex.*;
public class So {
    private static BufferedReader br;
    private static int n,index,t,noi=0;
    private static int []arr;
    private static int []ans;
    public static void main(String[] args)throws IOException {
        int i,k=1;int s,st,j;
        br=new BufferedReader(new InputStreamReader(System.in));
        String []str;
        t=Integer.parseInt(br.readLine());
        ans=new int[t];
        for(i=0;i<t;++i)
        {
         noi=0;
         n=Integer.parseInt(br.readLine());
         arr=new int[n];
         str=br.readLine().split(" ");
         for(k=0;k<n;++k)
          arr[k]=Integer.parseInt(str[k]);
         ms(0,n-1);
         ans[i]=noi;
        }
        for(int e:ans)
         System.out.println(e);
    }
    public static void ms(int l,int r)
    {
        if(r-l>0)
        {
            int mid=(l+r)/2;
            ms(0,mid);
            ms(mid+1,r);
            merge(l,r);
        }
    }
    public static void merge(int l,int r)
    {
        int []arr2=new int[r-l+1];
        int j,k,mid=(l+r)/2;
        k=mid+1;int i=0;
        j=l;
        while(j<=mid&&k<=r)
        {
            if(arr[j]<=arr[k])
             arr2[i++]=arr[j++];
            else {arr2[i++]=arr[k++];noi+=mid-j+1;}
        }
        System.arraycopy(arr,j,arr2,i,mid-j+1);
        System.arraycopy(arr,k,arr2,i,r-k+1);
        System.arraycopy(arr2,0,arr,l,r-l+1);
    }
}

The timeout is caused by a bug is in the ms method.  ms(0,mid)  should be ms(l, mid).

A few comments on the code itself:

Don't use lower case L as a variable name.  It looks too similar to the digit 1.
Don't use static variables to communicate between your methods.  It makes the code harder to test and you risk bugs due to crosstalk between different invocations of the method.  In this case, for example, ms and merge should just return the number of shifts instead of incrementing static counter noi
Better separation of input parsing from actual computation.  You should have a method long countingMergeSort(int[] ary).  That would make it easier for you to test your code.


Assembly line scheduling This is solution to the assembly line problem.  Here is complete description, which is too big and verbose to re-describe. I apologize for the inconvenience of the hyperlink.

Note: I do understand merits of unit testing in separate files. But deliberately added it to main method for personal convenience, so I request that you not consider that in your feedback.

I'm looking for request code review, optimizations and best practices.

final class Station {

    private final int stationTime;
    private final int transtionToTime; // time to transition`to` this station
    private final int transitionFromTime; // time to transitition `from` this station.

    public Station(int stationTime, int transitionTime, int transitionFromTime) {
        this.stationTime = stationTime;
        this.transtionToTime = transitionTime;
        this.transitionFromTime = transitionFromTime;
    }

    public int getStationTime() {
        return stationTime;
    }

    public int getTransitionToTime() {
        return transtionToTime;
    }

    public int getTransitionFromTime() {
        return transitionFromTime;
    }
}

public final class AssemblyLine {

    private AssemblyLine() {}

    /**
     * Given an assembly line, outputs the shortest time needed to go through it.
     * The input data structures are not expected to be changed by client.
     *
     * @param line1     the assembly line 1
     * @param line2     the assumbly line 2
     * @return          the minumum time needed to process the item.
     */
public static int assemblyLineShortestTime(List<Station> line1, List<Station> line2) {
    int prevLine1Min = line1.get(0).getStationTime() + line1.get(0).getTransitionToTime();
    int prevLine2Min = line2.get(0).getStationTime() + line2.get(0).getTransitionToTime();
    for (int i = 1; i < line1.size(); i++) {
        int currentLine1Min = line1.get(i).getStationTime() + Math.min(prevLine1Min, prevLine2Min + line1.get(i).getTransitionToTime());
        int currentLine2Min = line2.get(i).getStationTime() + Math.min(prevLine1Min + line2.get(i).getTransitionToTime(), prevLine2Min);

        prevLine1Min = currentLine1Min;
        prevLine2Min = currentLine2Min;
    }
    return Math.min(prevLine1Min + line1.get(line1.size() - 1).getTransitionFromTime(), prevLine2Min + line2.get(line2.size() - 1).getTransitionFromTime());
}

    public static void main(String[] args) {

        List<Station> line1 = new ArrayList<Station>();
        line1.add(new Station(4, 10, 7));
        line1.add(new Station(5,  9, 4));
        line1.add(new Station(3,  2, 5));
        line1.add(new Station(2,  8, 18));

        List<Station> line2 = new ArrayList<Station>();
        line2.add(new Station( 2, 12, 9));
        line2.add(new Station(10,  7, 2));
        line2.add(new Station( 1,  4, 8));
        line2.add(new Station( 4,  5, 7));

        assertEquals(35, assemblyLineShortestTime(line1, line2));
    }
}

The code implementing the core algorithm is very cumbersome. This is partly due to shoehorning the line entry/exit times into the first/last stations as evidenced by some internal stations in your tests having unusable exit times. But the code duplication doesn't help either.

  Note: The diagram shows the transition time belonging to the station being transitioned from. Your code uses the transition time from the station on the other line. I'm going with the diagram as it seems cleaner to me.

Store the current stations into local variables to avoid having to read .get(0) and .get(i) many times. Also, when you name variables identically except for 1 and 2 (unavoidable in this method), don't hide the numbers in the middle of the names. Finally, since all these time-based local variables are for line X and track minimum times, drop Line and Min from their names.

public static int assemblyLineShortestTime(List<Station> line1, List<Station> line2) {
    return assemblyLineShortestTime(line1.iterator(), line2.iterator());
}

private static int assemblyLineShortestTime(Iterator<Station> line1, Iterator<Station> line2) {
    Station station1 = line1.next();
    Station station2 = line2.next();
    int prev1 = station1.getStationTime() + station1.getTransitionToTime();
    int prev2 = station2.getStationTime() + station2.getTransitionToTime();
    while (line1.hasNext()) {
        station1 = line1.next();
        station2 = line2.next();
        int curr1 = station1.getStationTime() + Math.min(prev1, prev2 + station2.getTransitionToTime());
        int curr2 = station2.getStationTime() + Math.min(prev2, prev1 + station1.getTransitionToTime());

        prev1 = curr1;
        prev2 = curr2;
    }
    return Math.min(prev1 + station1.getTransitionFromTime(), prev2 + station2.getTransitionFromTime());
}

While the code duplication seems much worse with those temporary variables, the logic is more apparent to me. The real problem is that there's no logic inside Station. If you add the line number to Station and added some helper methods, you could clean up that code a bit.

private static int assemblyLineShortestTime(Iterator<Station> line1, Iterator<Station> line2) {
    Station station1 = line1.next();
    Station station2 = line2.next();
    int prev1 = station1.start();
    int prev2 = station2.start();
    while (line1.hasNext()) {
        station1 = line1.next();
        station2 = line2.next();
        int curr1 = station1.proceed(prev1, prev2);
        int curr2 = station2.proceed(prev2, prev1);
        prev1 = curr1;
        prev2 = curr2;
    }
    return Math.min(prev1 + station1.exit(), prev2 + station2.exit());
}

class Station {
    public int start() {
        return stationTime + transitionToTime;
    }

    public int proceed(thisLineTime, otherLineTime) {
        // The transitionToTime here is not the correct one,
        // but this demonstrates the technique
        return stationTime + Math.min(thisLineTime, otherLineTime + transitionToTime);
    }

    public int exit() {
        return transitionFromTime;
    }
}

These small refactorings improve the readability only a little, but they don't alter it significantly. The real gains will require expanding the model and moving away from a single, static method.

Starting Over

If you're going to model the stations with classes to solve this, go all in and model the lines and entry/exit times properly to avoid confusion.

class Station
    int runTime - time to process this station
    int switchTime - time to switch lines

class Line
    List<Station> stations
    int entryTime - time to enter this line
    int exitTime - time to exit this line

Next model the location along the lines.

class Location
    Line line - which line are we on
    Station station - which station did we just go through
    int totalTime - elapsed time to reach this location

This design will allow moving the bulk of the algorithm into the model. You may not even need to model the lines themselves if you store the next stations in each station.

class Station
    int runTime - time to process this station
    Station next - next station on the same line
    Station otherNext - next station on the other line
    int switchTime - time to switch lines


Parse heterogenous JSON array I need to parse a JSON generated by iTunes Search API. I'm looking for all apps by a certain developer, and I usually get a response like this (simplified):

{
    "resultCount":4,
    "results": [
                   {
                       "wrapperType":"artist",
                       "artistType":"Software Artist",
                       "artistName":"Vladimir Kelin"
                   },
                   {
                       "wrapperType":"software",
                       "formattedPrice":"Free",
                       "trackId":933241656
                   },
                   {
                       "wrapperType":"software",
                       "formattedPrice":"$1.99",
                       "trackId":1175807581
                   }
                ]
}

As you can see, the results array is heterogenous, meaning it contains objects of 2 different types: artist and software.

I created a structure, implementing a new handy Codable protocol:

struct App: Codable {
    let trackId: Int
    let formattedPrice: String?

    /// - Parameter jsonDict: A dictionary produced by JSONSerialization
    init?(jsonDict: [String: Any]) {
        guard let trackId = jsonDict["trackId"] as? Int else { return nil }
        self.trackId = trackId

        formattedPrice = jsonDict["formattedPrice"] as? String
    }
}

I don't want to parse artist data, so I didn't created any structures for that.

It's relatively easy to parse such JSON using JSONSerialization:

do {
    let root = try unwrap(JSONSerialization.jsonObject(with: receivedData, options: []) as? [String: Any])
    let results = try unwrap(root["results"] as? [[String: Any]])
    var apps = [App]()
    for result in results {
        // Check "software" or "artist"
        guard result["wrapperType"] as? String == "software" else { continue }
        guard let app = App(jsonDict: result) else { continue }
        apps.append(app)
    }
    completion(.ok, apps)
} catch let e {
    let dataStr = String(bytes: receivedData, encoding: .utf8) ?? ""
    print("Exception: \"\(e)\", while parsing data: \"\(dataStr)\"")
    completion(.parseError, nil)
}

Do you see the unwrap() function for the first time?

The Problem. I'm looking for a way to parse the response using wonderful JSONDecoder class introduced in Swift 4. If the results array were homogenous I would be able to parse it in just few strings of code.

// Helper struct
struct TunesResponse: Codable {
    let resultCount: Int
    let results: [App]
}

// Parsing
let jsonDecoder = JSONDecoder()
if let parsedResponse = jsonDecoder.decode(TunesResponse.self, from: receivedData) {
    let apps = parsedResponse.results
    completion(.ok, apps)
} else {
    completion(.parseError, nil)
}

In practice I spend a lot of time but didn't found any way to apply JSONDecoder for such kind of response.

First of all, the models need to be defined, conforming to the Decodable protocol. Starting with the base class and then the subclass, see the example below:

class FieldDefinition : NSObject, Codable {
    let type: String

    enum CodingKeys: String, CodingKey {
        case type
    }

    required init(from decoder: Decoder) throws {
        let container = try decoder.container(keyedBy: CodingKeys.self)
        self.type = try container.decode(String.self, forKey: .type)
    }

    func encode(to encoder: Encoder) throws {
        var container = encoder.container(keyedBy: CodingKeys.self)
        try container.encode(self.type, forKey: .type)
    }
}

class TextFieldDefinition: FieldDefinition {
    let maxLength: Int

    enum TextCodingKeys: String, CodingKey {
        case maxLength
    }

    required init(from decoder: Decoder) throws {
        let container = try decoder.container(keyedBy: TextCodingKeys.self)
        self.maxLength = try container.decode(Int.self, forKey: .maxLength)
        try super.init(from: decoder)
    }

    override func encode(to encoder: Encoder) throws {
        var container = encoder.container(keyedBy: TextCodingKeys.self)
        try container.encode(self.maxLength, forKey: .maxLength)
        try super.encode(to: encoder)
    }
}

And here comes the tricky part. Next we create an enum DemoClassKeys, which is used to decode the dictionary containing the array of fields.

We also define an enum FieldDefinitionTypeKey which is used to serialize the type field (wrapperType in your case) for each object in the array.
And a final enum FieldDefinitionTypes with specifies all the possible FieldDefinition types.

With all the enums specified we can get to the serialization :

class DemoClass: Decodable {
    var fieldDefinitions: [FieldDefinition]

    enum DemoClassKeys: String, CodingKey {
        case fieldDefinitions
    }

    enum FieldDefinitionTypeKey: CodingKey {
        case type
    }

    enum FieldDefinitionTypes: String, Decodable {
        case textFieldDefinition = "TextFieldDefinition"
        case deterministicFieldDefinition = "DeterministicFieldDefinition"
    }

    required init(from decoder: Decoder) throws {
        let container = try decoder.container(keyedBy: DemoClassKeys.self)
        var arrayOfFieldType = try container.nestedUnkeyedContainer(forKey: DemoClassKeys.fieldDefinitions)
        var fieldDefinitions = [FieldDefinition]()

        var tmp = arrayOfFieldType
        while(!arrayOfFieldType.isAtEnd) {
            let fieldDefinition = try arrayOfFieldType.nestedContainer(keyedBy: FieldDefinitionTypeKey.self)
            let type = try fieldDefinition.decode(FieldDefinitionTypes.self, forKey: FieldDefinitionTypeKey.type)
            switch type {
            case .textFieldDefinition:
                fieldDefinitions.append(try tmp.decode(TextFieldDefinition.self))
            case .deterministicFieldDefinition:
                fieldDefinitions.append(try tmp.decode(DeterministicFieldDefinition.self))
            }
        }
        self.fieldDefinitions = fieldDefinitions
    }
}

And Here is the full working Demo:

var json = """
{
    "fieldDefinitions": [
        {
            "type": "TextFieldDefinition",
            "maxLength": 5
        },
        {
            "type": "DeterministicFieldDefinition",
            "defaultValue": "VISA"
        }
    ]
}
"""

let jsonDecoder = JSONDecoder()
do {
    let results = try jsonDecoder.decode(DemoClass.self, from: json.data(using: .utf8)!)
    for result in results.fieldDefinitions {
        print(result.self)
    }
} catch {
    print("caught: \(error)")
}


Using unique_ptr in FreeRTOS I am trying to implement smart pointers for our project.
We're using FreeRTOS which is written with a C API so it was quite a challenge to implement sending smart pointers around but somehow I managed it by combining casts, new calls (dont really like that..) move semantics and some other tricks.

On the surface it behaves as expected, but I want to be sure there is not any memory leak.
Furthermore, is there a way to use make_unique together with new? I have not really found a way yet.

Here is the code, it should work with an online C++ compiler (or use this link). It would be great if an expert could look over this. Maybe some things can be optimized, or maybe this code is not save at all and I just dont see it.

// Example program
#include <iostream>
#include <string>
#include <memory>
#include <cstring>

class DummyHelper {
public:
    DummyHelper(uint8_t someResource): someResource(someResource) {}

    virtual ~DummyHelper() {
       someResource = 0;
       std::cout << "Dtor called" << std::endl;
    }

    int getResource() {
       return someResource;
    }
private:
   int someResource = 0;
};

class CommandMessage {
public:
    CommandMessage() = default;
    virtual~ CommandMessage() {};

    void setParameter(uint64_t parameter) {
        memcpy(rawContent.data() + 4, &parameter, sizeof(parameter));
    }

    uint64_t getParameter() const {
       uint64_t param;
       memcpy(&param, rawContent.data() + 4, sizeof(param));
       return param;
    }

private:
    std::array<uint8_t, 12> rawContent;
};

void setTestMessage(CommandMessage *testMsg,
       uint8_t someResource) {
  auto unique_pptr = new std::unique_ptr<DummyHelper>(new DummyHelper(someResource));
  uint64_t test_ptr = reinterpret_cast<uint64_t>(unique_pptr);
  testMsg->setParameter(test_ptr);
}

std::unique_ptr<DummyHelper> getUniquePtr(
       CommandMessage *testMsg) {
   uint64_t raw_ptr = testMsg->getParameter();
   auto unique_pptr = reinterpret_cast<std::unique_ptr<DummyHelper>*>(raw_ptr);
   auto unique_ptr = std::move(*unique_pptr);
   delete unique_pptr;
   return unique_ptr;
}

int main()
{
  auto storagePtr = std::make_unique<DummyHelper>(42);
  CommandMessage testMessage;
  std::cout << "Setting message" << std::endl;
  setTestMessage(&testMessage, 42);
  // a message has been set and can be sent, e.g. via a message queue

  // in another class, the pointer is retrieved.
  auto unique_ptr = getUniquePtr(&testMessage);
  DummyHelper & test = *unique_ptr;
  std::cout << test.getResource() << std::endl;
  unique_ptr.reset();
}

Overview:
In relation to the message queue:

Since you are doing a manual new and delete that seems a bit of a waste.
Also the only thing you are creating is a unique_ptr.
The unique_ptr is never in stored correctly typed so it never does any work.

So I would not store a pointer to a std::unique_ptr I would just store the original pointer. I like the idea of passing in and retrieving a unique_ptr to the access function.

Why are you passing a pointer to CommandMessage? It can't be null (you never check so must be valid. To make this more formal pass by reference to indicate to the user that it should be a normal object.
void setTestMessage(CommandMessage *testMsg, uint8_t someResource)
std::unique_ptr<DummyHelper> getUniquePtr(CommandMessage *testMsg)

// I would do this:

void setTestMessage(CommandMessage& testMsg, uint8_t someResource)
std::unique_ptr<DummyHelper> getUniquePtr(CommandMessage& testMsg)

Note: Putting the '*' on the right by the variable is very "C" like. C++ considers this part of the type information so it is usually on the left with the type.

Why is the input message different from the output message?
void setTestMessage(CommandMessage *testMsg, uint8_t someResource)
std::unique_ptr<DummyHelper> getUniquePtr(CommandMessage *testMsg)

So input is a uint8_t but the output is std::unique_ptr<DummyHelper>? I would expect the interface to be symmetric.
template<typename T>
void sendMessage(CommandMessage& messageStream, std::unique_ptr<T>&& msg);

template<typename T>
std::unique_ptr<T> recvMessage(CommandMessage& messageStream);

template<typename T>
std::unique_ptr<T> recvMessage(CommandMessage& messageStream)
{
   // To store pointers in an integer type you should be using `std::intptr_t`
   // This is an int type that is guaranteed large enough to store a pointer.
   // Don't make the assumption that `uint64_t` is large enough.
   // It might be but that is not guaranteed for the future.
   std::intptr_t raw_ptr = messageStream.getParameter();

   return reinterpret_cast<T*>(raw_ptr);
}

template<typename T>
void sendMessage(CommandMessage& messageStream, std::unique_ptr<T>&& msg)
{
    messageStream.setParameter(msg.release());
}


Manipulating CSV files to regular text file At my job, often I have to manually convert *.csv Files structured like:

number,number\n
number,number\n
number,number\n
...

To a regular *.txt file with the structure:

#1
double filename(number of columns, number of columns)
 number number
 number number
 number number
 ...

This file is an input to another program, and rather than spending the time to copy and paste the excel sheet into notepad and crop out all the commas and make sure there are spaces and such I decided to write a python script that would do it for me.

Now, to be fair, I am new to Python, and this script works perfectly (first try), I would just like some feedback on my methods, or ways to improve my quality of code.

# A simple program to create a formatted text file from a *.csv file.

csv_file = input('Enter the name of your input file: ')
txt_file = input('Enter the name of your output file: ')

try:
    my_input_file = open(csv_file, "r")
except IOError as e:
    print("I/O error({0}): {1}".format(e.errno, e.strerror))

if not my_input_file.closed:
    text_list = [];
    for line in my_input_file.readlines():
        line = line.split(",", 2)
        text_list.append(" ".join(line))
    my_input_file.close()

try:
    my_output_file = open(txt_file, "w")
except IOError as e:
    print("I/O error({0}): {1}".format(e.errno, e.strerror))

if not my_output_file.closed:
    my_output_file.write("#1\n")
    my_output_file.write("double({},{})\n".format(len(text_list), 2))
    for line in text_list:
        my_output_file.write("  " + line)
    print('File Successfully written.')
    my_output_file.close()

Use with when dealing with files.

There are great benefits discussed in the docs. Summarizing it handles closing and exceptions automatically. It makes it possible to write shorter code: look Andrey's answer for a rewrite.

Miscellaneous

Drop my from the start of your variables' names.
Learn list comprehension because it is extremely powerful, here I give you an example:

:

with open(csv_file) as input_file:
    lines = [line.split(",", 2) for line in input_file.readlines()]
    text_list = [" ".join(line) for line in lines]


Cosine simularity perfomance c# I am using 2 nested SortedDictionaries to construct sparse matrix

Here is the custom simularity(sim) function I wrote . Now it has O(n^2) complexity. I looking for suggestions to improve robustness and efficiency.Thanks for any help.

       double
            a = 0, b = 0,
            sqrta = 0,
            sqrtb = 0,
            sim = 0;

        foreach (var word_i in dict)
        {
            foreach (var word_j in dict)
            {
                if (word_i.Key == word_j.Key) continue;

                sim=a=b=sqrta=sqrtb=0;
                foreach (var term in word_j.Value.Keys)
                {
                    if (word_i.Value.ContainsKey(term))
                    {
                        word_i.Value.TryGetValue(term,out a);
                        word_j.Value.TryGetValue(term,out b);
                        sim += a * b;
                        sqrta += Math.Pow(a,2);
                        sqrtb += Math.Pow(b,2);
                    }

                }

                sim /= Math.Sqrt(sqrta) * Math.Sqrt(sqrtb);

            }
        }

As you are doing calculations on all values where the result depends on both loops, there isn't much that can be done about the complexity, at least not without knowing what you do with the result (which seems to be just discarded in the code shown).

There are some things that you can do in the innermost loop:

Instead of first using Contains and then get the value, you can use TryGetValue directly.

You know that term exists in the other collection, so you don't need TryGetValue for that one.

Squaring is done a lot faster by just multiplying instead of using Math.Pow.

if (word_i.Value.TryGetValue(term,out a)) {
  b = word_j.Value[term];
  sim += a * b;
  sqrta += a * a;
  sqrtb += b * b;
}


Counting occurrences of a character in a string Scenario "The Semantic Mind-Reader!" browser known as jhool is known
  as mind-reader for a reason. Here's why:

  You don't need to type 'www.' to open a website anymore. Though, you
  still need to type '.com' to open a website. The browser predicts ALL
  THE VOWELS in the name of the website. (Not '.com', though. Again!)
  Obviously, this means you can type the name of a website faster and
  save some time.

  Input format: The first line contains tc, the number of test cases. The second line contains the name of websites, as a string.

  Output format: You have to print the ratio of characters you would have typed in Jhool's browser, to your normal browser.

Constraints:
1 <= tc <= 100
1 <= Length of the website <= 200

  NOTE: You do NOT need to print the output in its lowest format. You
  should print in its original fraction format. The names of all the
  websites will be in small case only.

  Every string will start from www. and end with .com, so well!

  Sample Input

 2
www.google.com
www.hackerearth.com

  Sample Output

7/14
11/19

  Explanation
  Consider the first case: In Jhool's browser, you'll only have to type: ggl.com (7 characters) while in a normal browser, you'll have to
  type www.google.com, which is 14 characters.

Here is my solution:

import java.util.Scanner;
class TestClass {
    public static void main(String args[] ) throws Exception {
        Scanner keyboard=new Scanner(System.in);
        int t=keyboard.nextInt();
        keyboard.nextLine();
        while(t!=0){

            String s= keyboard.nextLine();

            int len=s.length();
            int count=0;
            for(int i=4;i<len-4;i++){
                if(s.substring(i,i+1).matches("[aeiouAEIOU]"))
                    count++;
            }
            System.out.println(len-count-4+"/"+len);
            t--;
        }
    }
}

But the above took around 2.4583 sec in total for against the 7 set of input.

input 1 input 2 input 3 input 4 input 5 input 6 input 7

Which I was able to further reduce to 1.9038 sec with this solution:

import java.util.ArrayList;
import java.util.Scanner;
class TestClass {
    public static void main(String args[] ) throws Exception {
        Scanner keyboard=new Scanner(System.in);
        int t=keyboard.nextInt();
        keyboard.nextLine();
        ArrayList<String> vowelList= new ArrayList<>();
        vowelList.add("a");
        vowelList.add("e");
        vowelList.add("i");
        vowelList.add("o");
        vowelList.add("u");
        while(t!=0){
            String s= keyboard.nextLine();
            int len=s.length();
            String sub=s.substring(4,len-4);
            String[] splitted=sub.split("\\B");
            ArrayList<String> splt= new ArrayList<>();
            for(String x:splitted){
                splt.add(x);
            }
            splt.removeAll(vowelList);

            int size=splt.size();
            System.out.println(size+4+"/"+len);
            t--;
        }
    }
}

Now if I'm using List<String> vowelList= new ArrayList<>(); and List<String> splt= new ArrayList<>(); instead of the above two ArrayList declaration above, I'm further able to reduce it to 1.8892 sec.

What are more possible ways of optimizing the above code which can reduce the running time? And why is their time difference using Implemented ArrayList call and through interface call of List? Or has it just happened without any reasons?

So, performance. Let's list off things that are commonly done wrong that you do in your code:

print statements in a loop
line-by-line parsing of input
mid-list removals of values (removeAll())
not enough functions

The performance can be significantly improved by doing batch operations on the input, processing, and output.

Additionally, the last point is important. Java compiles code essentially on a method-by-method basis. By keeping all your code in the main method you are reducing the possibility for Java to compile and optimize it.

Read the whole file in, split the lines out in one operation, write all the results out as one string.

That will likely remove more than half your execution time... but your actual algorithm is slow as well.

Since you use regular expressions to split the characters, you may as well use one to just clean up the whole URL... and strip all vowels that way.

Here's a scratch-up of what I would consider given this task. It shows the ideas I was mentioning, and also uses some Java 8 features:

public static void main(String[] args) throws IOException {
    for (String infile : args) {
        System.out.println(processSource(Paths.get(infile)));
    }
}

private static String processSource(Path path) throws IOException {
    List<String> alldata = Files.readAllLines(path);
    Iterator<String> it = alldata.iterator();
    final int size = Integer.parseInt(it.next());

    String[] results = new String[size];
    for (int cnt = 0; it.hasNext() && cnt < size; cnt++) {
        results[cnt] = computeRatio(it.next());
    }

    return String.join("\n", results);

}

private static final Pattern DISCARD = Pattern.compile("(^www\\.)|[aeiou]");

private static final String computeRatio(String url) {
    int limit = url.lastIndexOf('.');
    String clean = DISCARD.matcher(url.substring(0, limit)).replaceAll("");
    // include the URL for debug:
    // return String.format("%d/%d -> %s", clean.length() + url.length() - limit, url.length(), url);

    // simple result
    return String.format("%d/%d", clean.length() + url.length() - limit, url.length());
}


Separating logic and presentation in wordpress Recently, I've been thinking about better ways to make my WP themes more maintainable. What I've tried on one of my last projects is to separate out the presentation and logic as much as I can. First, I created a function with the loop in it and has all of the logic. It returns all of the data into an array. In the template page, I call the function, loop through the array, and output the HTML.

At the most basic level, this is what it looks like. In functions.php (or a plugin):

function get_data(){
    if (have_posts()) :
        while (have_posts()) : the_post();
            $results['title'] = get_the_title();
            $results['field'] = get_field('field');
        endwhile;
    endif;
    return $results;
}

In the template file:

query_posts( 'posts_per_page=5' );
$results = get_data();
foreach ($results as $result) :

echo $result['title'];

        echo $result['field'];

endforeach;

Is this a good practice? From what I can see, it's a good solution but I haven't seen any other developers use it. Is there something better that other people are doing?

Actually, there is something better, and that is using templates. Yes, it might be "radical" especially when you see no other developer near you use it. And yes, even my college instructors and friends don't know what templates are and still stuff logic with the presentation so I know how it feels as well.

I suggest you take a look at Mustache for starters. Basically, your templates will be logicless. The template will only contain a bit of custom markup. What your logic will only need to do is load the data, load the template, use the render function and it returns rendered markup, complete with the data.


Joining tables by skipping the intermediate parent table We've tables adm_app, quals_data to which personal_data is a parent table. We are retrieving data from another table sub_marks which has a foreign key of quals_data and the query is like this

select
      sub_marks.obtain_marks,
      sub_marks.convertion_marks,
      sub_marks.max_marks
from sub_marks
inner join quals_data ON quals_data.id = sub_marks.quals_data_id
inner join adm_app on quals_data.personal_data_id = adm_app.personal_data_id
where adm_app.app_no = 2000;

You can see that we've skipped the join with personal_data because we are not using its data anywhere. Is this a good way of writing a query? Previously we wrote query like this

select
      sub_marks.obtain_marks,
      sub_marks.convertion_marks,
      sub_marks.max_marks
from sub_marks
inner join quals_data ON quals_data.id = sub_marks.quals_data_id
inner join personal_data ON personal_data.id = quals_data.personal_data_id
inner join adm_app on adm_app.personal_data_id = personal_data.id
where adm_app.app_no = 2000;

Your changes to the query are good changes.

Just because the both adm_app and quals_data have the same child relationship to the personal_data table does not mean you have to include the personal_data table in a join if there's no relevant data on it.

You should run both of your queries with the query-plan EXPLAIN to show the difference in the queries. Avoiding the join will make for a simpler query with better performance.

Having said that, what about the rest of your query, are there improvements that can be made? Nothing big, really. I am not a big fan of snake_case for table and column names, but it is more important that you are consistent, and neat with the concept.

If I were to have any complaints, it would just be two really small things:

a spelling error.... I know, it's seriously pedantic, but in my "real job" I would point out the error soon because it is the sort of thing that is hard to correct later... convertion should be spelled conversion
abbreviations should be consistent in names. Since you have personal_data and fully named columns like convertion_marks, then it follows that you should not have abbreviations for adm_app and quals_data. Those should be full names.

As you can tell, the only issues I can see are really trivial.


Renaming and moving files in a subdirectory with an index and the folder name I'm putting together a script to take .mp4 files organized into folders that described the video (eg. cat) and I'd like to do the following:

rename them with the folder name and a number
move the video into a folder named and numbered in the same manner
parse the video into frames using ffmpeg

I'm posting this here to be a reference for others and to ask about how this could be done better.  It works fine but I'm wondering how to make it better.

for dir in /path/to/parent/directory/*; do
cd "$dir" && result=${PWD##*/}
cd video
echo "$result"
a=1
    for i in *.mp4; do
        dafile="$(printf "_%02d.mp4" ${a})"
        dafolder="$(printf "_%02d" ${a})"
        newfolder="$result$dafolder"
        newfile="$result$dafile"
        mkdir "${newfolder}"
        mv "${i}" "${newfolder}/${newfile}"

        cd "${newfolder}"
            ffmpeg -i "${newfile}" -r 30 -f image2 "${newfolder}-\%04d.png"
        cd ..
        let a=a+1
    done

done

I used the following links to figure out a bunch of ideas for the code:

Get current directory name without full path in Bash script
Renaming files in a folder to sequential numbers

Quite nicely done! I only have a few minor comments.

The script is not very "safe" as it is.
There are many possible failure points which are ignored:

What if /path/to/parent/directory/* doesn't exist?
What if cd "$dir" fails? ($dir is not really a directory, or you don't have permission to enter it)
What if cd video fails? (Like $dir)
What if *.mp4 doesn't exist?
What if mkdir "${newfolder}" fails? (no permission)

If any of the above happens,
the script will continue to run regardless of any errors,
and potentially create junk files and directories you didn't intend.

Here's a safer way to protect against these failures:

for dir in /path/to/parent/directory/*; do
    cd "$dir" || continue
    result=${PWD##*/}
    cd video || continue
    echo "$result"
    a=1
    for i in *.mp4; do
        test -f "$i" || continue
        dafile="$(printf "_%02d.mp4" ${a})"
        dafolder="$(printf "_%02d" ${a})"
        newfolder="$result$dafolder"
        newfile="$result$dafile"
        mkdir "${newfolder}" || continue
        mv "${i}" "${newfolder}/${newfile}"
        cd "${newfolder}"
        ffmpeg -i "${newfile}" -r 30 -f image2 "${newfolder}-\%04d.png"
        cd ..
        ((a++))
    done
done

I also fixed the indentation, and replaced let a=a+1 with the somewhat simpler and more intuitive ((a++)).

You seem to like ${name} instead of $name. That's fine, but it's not needed anywhere in the posted code. Usually it's needed in situations like ${name}_suffix, where without the ${...} the "_suffix" would become part of the variable name.


Rewriting timers in .NET with lambda var timeToWait = TimeSpan.FromSeconds(20);
var interval = TimeSpan.FromMinutes(5);
var t = new Timer(s =>
    {
        tracker.ProcessAuditLogs();
    }, null, timeToWait, interval);

I'm looking to run a timer or an indefinite period of time, as long as my program/service is running. Will the above work in a production environment?

I converted my timer loops to Reactive Extensions a while back to avoid the memory leaks created by event subscriptions and event handlers.  A snippet doing the same thing as yours is...

        var timer = Observable
            .Timer(TimeSpan.FromSeconds(3), TimeSpan.FromSeconds(3))
            .Subscribe(q =>
                {   // this executes every 3 seconds in a detached loop
                    Console.WriteLine("do something here " + q);
                });
        // other work goes here
        GC.KeepAlive(timer);

Of interest here is the last line.  This tells the GC that the object should not be garbage collected until it is reached.  You can use the same strategy for your event driven timer.

If you wanted a fascinating glimpse into the new garbage collector introduced in 4.5, Maoni Stephens (the lady who actually wrote the GC) made a video that's a 'must see'...  http://channel9.msdn.com/posts/Maoni-Stephens-CLR-45-Server-Background-GC


Fastest way to extract n bytes from uint8_t buffer I work a lot with byte buffers and have to extract different parts. In this example, it's 4 byte, but it ranges from a single bit to 128 bit. Speed is the most important metric here. See the code for a MWE. I'd like to know if there is a better way.

#include <stdint.h>

static uint32_t get_data(uint8_t *buf, size_t off)
{
    return ((uint32_t)(buf[off + 0]) << 24) +
                  ((uint32_t)(buf[off + 1]) << 16) +
                  ((uint32_t)(buf[off + 2])  <<  8) +
                  ((uint32_t)(buf[off + 3]));
}

int main(int argc, char **argv)
{
    uint8_t buf[128];

    /* get some example data */
    for (uint8_t i = 0; i < 128; ++i)
        buf[i] = i;

    /* we want the data from offset 10 as an uint32_t */
    uint32_t res = get_data(buf, 10);
}

Since you want low level operations, I'd suggest memmove

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

static uint32_t get_data(uint8_t *buf, size_t off)
{
    return ((uint32_t)(buf[off + 0]) << 24) +
                  ((uint32_t)(buf[off + 1]) << 16) +
                  ((uint32_t)(buf[off + 2])  <<  8) +
                  ((uint32_t)(buf[off + 3]));
}

int main(int argc, char **argv)
{
    uint8_t buf[128];

    /* get some example data */
    for (uint8_t i = 0; i < 128; ++i)
        buf[i] = i;

    clock_t t = clock();
    uint32_t res;
    for(int i=0; i<10000; i++)
        memmove(&res, buf+10, sizeof(uint32_t));
    t = clock() -t;
    printf("Time %lf\n", (double)t/CLOCKS_PER_SEC);

    t = clock();
    for(int i=0; i<10000; i++)
     res = get_data(buf, 10);
    t = clock() -t;
    printf("Time %lf\n", (double)t/CLOCKS_PER_SEC);
}

Because a single copy doesn't show any difference I tried with 10.000 and my results were:

Time 0.000049
Time 0.000090

Almost double the speedup

EDIT 1: As mentioned in the comments, memcpy is a viable alternative
to memmove.
EDIT 2: The speed difference in this example cannot be
observed with -O flag as the compiler executes the loop only one
time.


Are my forms a danger towards code injection? I'm writing a PHP tutorial and I would like to display some forms where the users could enter values that are displayed in the same webpage, just as a demonstration.

The forms do nothing special, they only use print instructions to display the input.

I would like to know if these apparently innofensive forms could be a real danger for my server because of script injection.

The code that processes the form is:

<?php
      if (array_key_exists('user', $_POST)) {
            print "Hello, " . $_POST['user'];
      } else {
            print "Waiting for your input...";
      }
?>

The Short answer is yes you are vulnerable to injection. XSS to be precise which you can read more about here https://www.owasp.org/index.php/XSS_(Cross_Site_Scripting)_Prevention_Cheat_Sheet

Explaination:

All user input should be sanatized for example:

if you input <script>alert("This will alert");</script> into your form you will notice an alert message will appear on your page

however if you sanatize the code i.e.

print "Hello, " . htmlentities($_POST['user']);

you will no longer see the alert message

using htmlentities() will help protect you from the script injection.

You would also be better validating the data that will be expected from the user

Other points which you can see here https://stackoverflow.com/questions/11554432/php-post-dynamic-variable-names-security-concerns which are based more on dynamically creating variables


Coalescing lvalue references to std-optionals I want to coalesce (lvalue references to) optionals of the same type, in C++17:

#include <optional>

template <typename... Ts>
constexpr auto coalesce(std::optional<Ts>&... optionals );

template <typename T>
constexpr std::optional<T>& coalesce(std::optional<T>& x, std::optional<T>& y)
{
    return x.has_value() ? x : y;
}

template <typename T>
constexpr std::optional<T> coalesce(std::optional<T>& x)
{
    return x;
}

template <typename T, typename... Ts>
constexpr std::optional<T> coalesce(std::optional<T>& x, std::optional<Ts>&... xs )
{
    return coalesce(x, coalesce(std::forward<std::optional<T>&>(xs)...));
}

This is sub-optimal, as it uses recursive templated function calls, rather than ellipsis-based expansion in a single body. What would you suggest?

Notes:

C++17 and no fancy libraries please.
This can be seen as a specific case of the "return first parameter in a pack satisfying a predicate", with the predicate being [](auto x) { return x.has_value(); }
If possible, let's not copy any T's.

So ideally you want to write something like:

template <typename... Ts>
constepxr auto coalesce(std::optional<Ts>&... xs)
{
   return (xs || ...);
}

But the issue here is of course that it will convert the optionals to bool before applying the boolean or-operator. You could however cast it to a type that does provide the right semantics for the boolean or-operator to do the thing you want:

template <typename T>
struct coalescable {
    T &x;
    coalescable(T &x_): x(x_) {}
    constexpr const coalescable &operator||(const coalescable &other) {
        return x.has_value() ? *this : other;
    }
};

And then use that in the fold expression:

template <typename... Ts>
constepxr auto coalesce(std::optional<Ts>&... xs)
{
   return (coalescable(xs) || ...).x;
}


Concurrent non-blocking refresh In my application there is a "config" object that is accessbile by multiple threads concurrently.
It is a single object (singleton) injected by the DI in the "dependents".
Until now, this (the "config") object was immutable which makes it inherently thread-safe.
However, there is a new requirement that the "config" should be periodically "refreshed/updated".
I wrote a wrapper (decorator) that refreshes itself (from a configProvider), and the refresh happens on the fly, when a method gets called (if the certian amount of time has passed)
The wrapper should be thread safe and non-blocking.

Any (concurrency) issues with the following code?

package xyz;

import java.util.concurrent.atomic.AtomicBoolean;

/**
 * Implementation of {@link IMyModuleConfig} that periodically refreshes itself from the provided
 * {@link IConfiguration}.
 * The refresh is done on-the-fly when some method is called, it is thread safe.
 *
 * @Threadsafe
 */
public class MyModuleRefreshableConfig implements IMyModuleConfig {

    private IMyModuleConfig configBean; // Decorated object, Immutable, non-final should be properly fenced
    private long lastRefreshTime;  // time when we refreshed the configuration last, non-final should be properly fenced

    private final IConfiguration configProvider; // refresh source
    private final long refreshInterval; // how often we refresh

    private final AtomicBoolean refreshConfigMutex; // if true, a thread is refreshing configuration right now

    public MyModuleRefreshableConfig(IConfiguration configProvider, long refreshInterval) {
        refreshConfigMutex = new AtomicBoolean(false);

        this.configBean = refreshConfig();
        this.lastRefreshTime = System.currentTimeMillis();

        // Do not re-arrange, the assignment to final field latter in the constructor serves as a "store fence"

        this.configProvider = configProvider;
        this.refreshInterval = refreshInterval;
    }

    @Override
    public boolean getMyBoolean() {
        refreshConfigIfNeeded(); // full fence
        return this.configBean.getMyBoolean();
    }

    @Override
    public int getTimeout() {
        refreshConfigIfNeeded(); // full fence
        return this.configBean.getTimeout();
    }

    private void refreshConfigIfNeeded() {
        boolean aquired = refreshConfigMutex.compareAndSet(false, true); // full fence
        if (aquired) { // critical section starts, if some thread is in just continue, no need to wait
            long cTime = System.currentTimeMillis();
            boolean shouldRefresh = (cTime - lastRefreshTime) > refreshInterval;
            if (shouldRefresh) {
                lastRefreshTime = cTime;
                configBean = refreshConfig();
            }
            refreshConfigMutex.set(false); // critical section ends, write fence
        }
    }

    private IMyModuleConfig refreshConfig() {
        // creates new immutable IMyModuleConfig with values provided by configProvider
    }
}

As long as you are 100% certain that all threads will use the same instance of MyModuleRefreshableConfig, this code should work. Just to be certain I would make the refreshConfigMutexstatic:

private static final AtomicBoolean refreshConfigMutex;

public MyModuleRefreshableConfig(IConfiguration configProvider, long refreshInterval) {
    synchronized {
         if (refreshConfigMutex == null) {
             refreshConfigMutex = new AtomicBoolean(false);
         }
    }
    ....
}


JS exercise to process an array of animals My background is primarily in C#. Though, I've gotten comfortable with JavaScript, I feel that stylewise my JS code looks more like C# code.

Does the following code's style conform to "standard JS style"?

processArray = (myArray) => {

   if(typeof myArray === "undefined" || myArray == null || myArray.length == 0)
      return null;

   let animals = [];
   let animal = {};

   for(let item of myArray) {

      animal = getAnimal(item);
      if(item !== null)
         animals.push(animal);
   };

   return animals;
}

getAnimal = (data) => {

   if(typeof data === "undefined" || data == null)
      return null;

   const animal = {
      id: data.id,
      type: data.type,
      name: data.name
   };

   return animal;
}

It's important to note that I'm not so much talking about formatting such as indentations, opening curly braces on the same line as the function name, etc.

What I'm really talking about is the way the code is structured. For example, as an "old school" C# developer, I like creating variables and assigning my data to them and cleanly returning them in my methods.

I see that a lot of JS developers chain one function to another or use a lot of callbacks. In other words, I see a lot of intermingling of functions. I think C# has also borrowed a few patterns from JS but for the most part, the coding style I used here is pretty typical "old school" C#.

Maybe an analogy will help clarify what I'm talking about. It's like a person whose native language is German/Italian/Japanase/etc. speaking English with an accent. Even if the grammar and even choice of words are perfect, the accent always gives it away.

So, I'm trying to understand if I'm writing JS code with a C# accent :-)

I do spot a minor "C# accent" :)

Unless you are dealing with very unruly data, an idiomatic approach would likely drop most of the special-case checks and use map instead of the for-loop:

// This function could even be removed, depending on the larger details of your code
function getAnimals(data) {
  return data.map(getAnimal);
}

function getAnimal(item) {
  return {
    id: item.id,
    type: item.type,
    name: item.name,
  };
}

If the source array may contain null-ey items, you could filter them out beforehand, instead of after attempting to process them:

function getAnimals(data) {
  return data
    .filter(Boolean)
    .map(getAnimal);
}

(null or undefined will evaluate to false in a boolean context.)

If you have a utility library like Lodash on hand, you could use a function like pick to further simplify things:

const getAnimal = item => _.pick(item, ['id', 'type', 'name']);

Otherwise, on a more general scale, most JS code tends to omit explicit null/undefined checks. Let the code throw an error if necessary, otherwise fix whatever is calling your function, eg. by filtering. There's a place for such checks, but in JS it's usually not in every function.

  What I'm really talking about is the way the code is structured. For example, as an "old school" C# developer, I like creating variables and assigning my data to them and cleanly returning them in my methods.

I don't think the code you've provided here merits any temporary variables. Idiomatically, it's a single function applied via Array#map.

Minor syntax things

Your functions are currently in the global namespace, because JS permits variables with no qualifiers as implicit globals. Prefix them with const or let to explicitly make them variables, or use function when possible since it plays nicer with hoisting:

const processArray = (myArray) => {...
// or, as above
function processArray(myArray) {...

Many people omit the braces around the single argument of an arrow function. It's optional, but reads slightly cleaner:

const processArray = myArray => {...

myArray is a toy name, I am assuming this is a toy example
Always use === for comparisons, which you do, unless you want to compare against both null and undefined, which you do, in which case you can do myArray == null. To go a bit further, your first conditional would more simply be written:

if (!myArray || !myArray.length) {

since null and undefined are both "falsey", and so is a zero length.

Essentially: your code works fine -- but it could be a lot shorter.


Bottom up mergesort - singly linked list I have written the following program in Go to mergesort a singly linked list. It does bottom up merge sort and hence does not have to find the mid of the linked list. Please review the algorithm and the code for idiomatic Go.

package main

import (
    "fmt"
)

type node struct {
    val  int
    next *node
}

type list struct {
    head *node
}

func newList(arr []int) *list {

    if arr == nil || len(arr) == 0 {
        return nil
    }
    l := new(list)
    l.head = getNode(arr[0])
    prev := l.head
    for i := 1; i < len(arr); i++ {
        n := getNode(arr[i])
        prev.next = n
        prev = n
    }

    return l
}

func printList(n *node) {
    for p := n; p != nil; p = p.next {
        fmt.Printf("%d ", p.val)
    }
}

func getNode(v int) *node {
    n := new(node)
    n.val = v
    return n
}

func skip(n *node, len int) (fast *node, slow *node) {

    counter := 0
    fast = n
    slow = n
    for counter < len {
        if fast != nil && fast.next != nil {
            fast = fast.next.next
        } else if fast != nil {
            fast = fast.next
        }

        if slow != nil {
            slow = slow.next
        }

        counter++
    }

    return fast, slow

}

func (l *list) mergesort() {

    // start with sublength of 1
    // compare 0,with 1, 2 with 3
    // skip n.next.next for next bunch

    sublen := 1
    len := 0
    counted := false

    for !counted || sublen < len {
        for n := l.head; n != nil; {
            fast, slow := skip(n, sublen)

            merge(n, slow, sublen)

            if !counted {
                len += 2
            }
            n = fast
        }
        counted = true
        sublen += sublen
    }
}

func merge(p *node, q *node, len int) {

    i := p

    if q == nil {
        // nothing to merge, return
        return
    }

    j := q
    ilen := 0
    jlen := 0

    var head *node
    var prev *node
    var end *node
    var penult *node

    for (i != q && ilen < len) && (j != nil && jlen < len) {

        if i.val < j.val {
            n := getNode(i.val)
            append(n, &head, &prev)
            i = i.next
            penult = prev
            prev = n
            ilen++
        } else {
            n := getNode(j.val)
            append(n, &head, &prev)
            setEnd(&end, j, jlen, len)
            j = j.next
            penult = prev
            prev = n
            jlen++
        }
    }

    for i != q && ilen < len {
        n := getNode(i.val)
        append(n, &head, &prev)
        i = i.next
        penult = prev
        prev = n
        ilen++
    }
    for j != nil && jlen < len {
        n := getNode(j.val)
        append(n, &head, &prev)
        setEnd(&end, j, jlen, len)
        j = j.next
        penult = prev
        prev = n
        jlen++
    }

    // replace into original list
    p.val = head.val
    end.val = prev.val

    // do we have any nodes between head and new end (prev) or is it just the
    // two nodes (head and new end (prev)?
    if head != penult {
        p.next = head.next
        penult.next = end
    }
}

func setEnd(end **node, n *node, partlen int, len int) {

    if n.next != nil && partlen < len-1 {
        *end = n.next
    } else {
        *end = n
    }
}

func append(n *node, head **node, prev **node) {

    if *head == nil {
        *head = n
        *prev = n
    } else {
        (*prev).next = n
    }
}

func main() {

    l := newList([]int{10, 4, 2, 15, 8, 9, 12, 1, 6})
    fmt.Println("Original: ")
    printList(l.head)
    l.mergesort()
    fmt.Println("\nSorted:")
    printList(l.head)

}

Also shared here

Just some random notes:

l := new(list)
l.head = getNode(arr[0])

This is usually spelled l := &list{head: getNode(arr[0])}.

n := new(node)
n.val = v

Again, can be shortened to n := &node{val: v}.

var head *node
var prev *node
var end *node
var penult *node

Shorten to var head, prev, end, penult *node.

func append

This is a very bad name for a function because it clashes with a built-in. Better change to something like appendList or make it a method.


Serialize and deserialize binary tree with a lot of empty nodes Here is my code to serialize and deserialize binary tree, which may have a lot of empty nodes. I am written in Python 2.7. Any advice on performance improvement in terms of algorithm time complexity, bugs or code style advice is appreciated.

My major idea is, serialize only non-empty nodes, with node ID information, where left node ID * 2 = parent node ID, and right node ID * 2 + 1 = parent node ID

Source code in Python 2.7,

from collections import defaultdict
class TreeNode:
    def __init__(self, value, left, right):
        self.value = value
        self.left = left
        self.right = right
    def serialize_tree(self, result):
        stack = []
        stack.append((self, 1))
        while len(stack) > 0:
            node, index = stack.pop(0)
            result.append((node.value, index))
            if node.left:
                stack.append((node.left, index*2))
            if node.right:
                stack.append((node.right, index*2+1))
    @staticmethod
    def deserialize_tree(source):
        result = []
        for (node_value, index) in source:
            node = TreeNode(node_value, None, None)
            result.append(node)
            if index == 1:
                continue
            elif index % 2 == 0:
                result[index / 2 - 1].left = node
            else:
                result[(index - 1) / 2 - 1].right = node
        return result[0]
    def print_tree(self, result):
        result.append(self.value)
        if self.left:
            self.left.print_tree(result)
        else:
            result.append('#')
        if self.right:
            self.right.print_tree(result)
        else:
            result.append('#')

if __name__ == "__main__":
    root = TreeNode(1, TreeNode(2, None, TreeNode(4, None, None)), TreeNode(3, None, None))
    result = []
    root.serialize_tree(result)
    print result
    new_root = TreeNode.deserialize_tree(result)
    result2 = []
    new_root.print_tree(result2)
    print result2

I would use a map mapping each key of a node \$u\$ in the tree to a 2-tuple containing the key of the parent \$p\$, and a boolean indicating whether \$u\$ is a left or right child of \$p\$:

class TreeNode:

    def __init__(self, key):
        self.key = key
        self.left = None
        self.right = None

    def __eq__(self, other):
        return self.key == other.key

    def __hash__(self):
        return self.key

def serialize_tree(root_node):
    map_node_key_to_parent_data = {}

    def serialize_tree_impl(node, parent_node):
        if parent_node:
            parent_data = (parent_node.key, parent_node.left is node)
            map_node_key_to_parent_data[node.key] = parent_data
        else:
            map_node_key_to_parent_data[node.key] = None

        if node.left:
            serialize_tree_impl(node.left, node)

        if node.right:
            serialize_tree_impl(node.right, node)

    serialize_tree_impl(root_node, None)
    return map_node_key_to_parent_data

def deserialize_tree(map_node_key_to_parent_data):
    map_node_key_to_tree_node = {}

    for node_key in map_node_key_to_parent_data.keys():
        if node_key not in map_node_key_to_tree_node.keys():
            map_node_key_to_tree_node[node_key] = TreeNode(node_key)
        else:
            raise TypeError("Duplicate node in the tree.")

    root_node = None

    for node_key, parent_data in map_node_key_to_parent_data.items():
        node = map_node_key_to_tree_node[node_key]

        if not parent_data:
            root_node = node
        else:
            parent_node_key = parent_data[0]
            on_left = parent_data[1]
            parent_node = map_node_key_to_tree_node[parent_node_key]

            if on_left:
                parent_node.left = node
            else:
                parent_node.right = node

    return root_node

def binary_search_trees_are_identical(root_node1, root_node2):
    if not root_node1 and not root_node2:
        return True

    if not root_node1 or not root_node2:
        return False

    if root_node1.key != root_node2.key:
        return False

    return binary_search_trees_are_identical(root_node1.left, root_node2.left)\
        and binary_search_trees_are_identical(root_node2.right, root_node2.right)

def main():
    root_node = TreeNode(5);
    l = TreeNode(3)
    r = TreeNode(7)
    rr = TreeNode(10)
    lr = TreeNode(4)
    rrl = TreeNode(11)
    rr.left = rrl

    root_node.left = l
    root_node.right = r
    l.right = lr
    r.right = rr

    serialized_tree = serialize_tree(root_node)
    print serialized_tree

    deserialized_tree_root_node = deserialize_tree(serialized_tree)
    print "Trees are identical:", binary_search_trees_are_identical(root_node, deserialized_tree_root_node)
    pass

if __name__ == "__main__":
    main()

A bug?

On

root = TreeNode(1, TreeNode(2, TreeNode(3, TreeNode(4, TreeNode(5, None, None), None), None), None), None)

I get an IndexError.

Hope that helps.


Displaying random "ads" from an XML document I have an XML document that contains "ads" and includes information about the department name, image name, link to URL, and alt text.  I have a PHP function that accepts two arguments, department name and the number of "ads" to display.  It reads through the entire XML document, and stores the information in arrays if the department name matches what is passed to the argument or matches  "all".  What I have works, but it seems rather long and cumbersome.  Is there a way to shorten it and improve performance?

PHP

function rightAds($deptName, $displayNumber) {
  //load xml
  /* --- Original loading, left in for completeness
  $completeurl = "http://example.com/example.xml";
  $xml = simplexml_load_file($completeurl);
  --- */

  //Loading without an HTTP call, as per Corbin's point
  $xml = simplexml_load_file(dirname(__FILE__)."/smallAdsByDepartment.xml");

  $smallAds = $xml->smallAds;

  //number of ads that can be used for department
  $numberForDepartment = 0;

  //loop through xml document, save the ads that are "all" or the department name
  foreach($smallAds as $ad) {

    //get department name
    $departmentName = $ad->departmentName;

    //if it is a valid department, add info to corresponding arrays
    if((strpos($departmentName, $deptName) !== false) || (strpos($departmentName, "all") !== false)) {

      //PT exception.  If department name includes noPT, and provided name is pt, don't include the image
      if(($deptName == "pt" && strpos($departmentName, "noPT") !== false) || (strpos(dirname($_SERVER['PHP_SELF']), "pt") !== false && basename($_SERVER['REQUEST_URI']) == "admissions.php" &&  strpos($departmentName, "noPT") !== false )) {
        continue;
      }
      //CLS exception.
      if(($deptName == "cls" && strpos($departmentName, "noCLS") !== false)) {
        continue;
      }

      $name[$numberForDepartment] = $ad->name;
      $alt[$numberForDepartment] = $ad->alt;
      $linkTo[$numberForDepartment] = $ad->linkTo;
      $numberForDepartment++;
    }
  }

  //if $displayNumber is 1, or an invalid negative number, display random ad.
  if($displayNumber <= 1) {
    $randKeys = mt_rand(0, $numberForDepartment);
    echo "

";
  }
  else {
        //get a list of randomly selected keys from the array
        $randKeys = range(0, (count($name) - 1));
        shuffle($randKeys);

        $numberOfAds = 0;

        //loop through and display image/link for each
        foreach($randKeys as $adNumber) {
            echo "

";
            $numberOfAds++;
            if($numberOfAds == $displayNumber) { break; }
        }
   }
}

Example XML

<smallAds>
    <departmentName>all</departmentName>
    <name>smallAd3.jpg</name>
    <alt>Faculty Research</alt>
    <linkTo>http://example.com/research/index.php</linkTo>
</smallAds>
<smallAds>
    <departmentName>all, noPT</departmentName>
    <name>smallAd6.jpg</name>
    <alt>Advising</alt>
    <linkTo>http://example.com/advising.php</linkTo>
</smallAds>
<smallAds>
    <departmentName>all, noCLS</departmentName>
    <name>checkYouTube.png</name>
    <alt>Check us out on YouTube</alt>
    <linkTo>http://www.youtube.com/user/example</linkTo>
</smallAds>

If you can use any format, use serialized PHP, it's by far the fastest.

<?php

class Ad {

  public $alt;

  public $departmentName;

  public $linkTo;

  public $name;

}

function formatAd(Ad $ad) {
  return "
linkTo}'>name}' alt='{$ad->alt}'>
";
}

function rightAds($departmentName, $displayNumber) {
  static $ads = null;

  if (!$ads) {
    $ads = unserialize(file_get_contents(__DIR__ . "/smallAdsByDepartment.ser"));
  }

  $pt = $departmentName === "pt";

  /* @var $ad Ad */
  foreach ($ads as $delta => $ad) {
    if (strpos($ad->departmentName, $departmentName) !== false || strpos($departmentName, "all") !== false) {
      $noPT = strpos($ad->departmentName, "noPT") !== false;
      if (($pt && $noPT) || (strpos(__DIR__, "pt") !== false && basename($_SERVER["REQUEST_URI"]) === "admissions.php" && $noPT) || ($departmentName === "cls" && strpos($ad->departmentName, "noCLS") !== false)) {
        unset($ads[$delta]);
      }
    }
  }

  if ($displayNumber <= 1) {
    echo formatAd($ads[mt_rand(0, count($ads) - 1)]);
  }
  else {
    $keys = range(0, count($ads) - 1);
    shuffle($keys);
    $c = count($keys);
    for ($i = 0; $i < $c; ++$i) {
      echo formatAd($ads[$keys[$i]]);
      if ($i === $displayNumber) {
        break;
      }
    }
  }
}


Return payload in service layer In my project I am using the ADR architecture, a branch of MVC.
In my actions (Controllers) I handle only data that comes from the request.
When there is a business rule that defines which status code I return, I do this treatment in my service.
Imagine this scenario:
When editing a user, I check if the user's email already exists, if it exists, I return an error informed to your message. This treatment is done in the service, and in these cases I get the error in it.
I would like to hear from you about my approach to, in some cases, the service layer returning the payload.
Action:
class CreateUserAction extends Action
{
    private $userService;

    /**
     * @param UserService $userService
     */
    public function __construct(UserService $userService)
    {
        $this->userService = $userService;
    }

    public function handle(Request $request) : JsonResponse
    {
        $data = $request->getData();

        $payload = $this->userService->create($data, $user);

        return response()->json($payload->body, $payload->statusCode);
    }
}

Service:
class UserService extends BaseService
{
    private $userRepository;

    public function __construct(UserRepository $userRepository)
    {
        $this->userRepository = $userRepository;
    }

    public function create(array $data) : object
    {
        if ($this->userRepository->findByEmail($data['email'])) {
            $this->setPayloadError(
                __('messages.user.contributor_email_already_exists'),
                Response::HTTP_BAD_REQUEST
            );
            return $this->getPayload();
        }

        $userCreated = $this->userRepository->create($data);

        $this->setPayload($userCreated, Response::HTTP_CREATED);
        return $this->getPayload();

    }
}
```

In my actions (Controllers) I handle only data that comes from the request.

If above is the case you might be better off handling validation before calling service class. In other words, prepare data into a format that business service classes can understand. Same applies about a decision on which HTTP Code to return.
I am not sure why you have single responsibility controllers, a number of files would bloat very quickly in a larger application.
Controller:
class CreateUserAction extends Action
{
    public function handle(UserCreateRequest $request): JsonResponse
    {
        return (new UserService)->create($request->validated(), $user);

        // OR
        $user = (new UserService)->create($request->validated(), $user);

        return response($user, Response::HTTP_CREATED);
    }
}

I am not sure why you are using repository unless you are doing complex DB operation. Service should have all the business logic. This way service layer/business logic can be easier extracted and reused in other applications thanks to less dependencies.
use App/Models/User;

class UserService extends BaseService
{
    public function create(array $data): User
    {
        return (new userRepository)->create($data);

    }
}


C++ Optional Implementation I haven't done much programming in C++, so I figured I'd try making an Optional class. I'm mainly concerned that I didn't follow good conventions or that I didn't use exceptions well. I know that there are no comments, but for now, let's ignore that.

The idea is that my Optional class can be used instead of pointers and nullptr when I need to return an optional value.

optional.h

#pragma once

namespace extra { // I'm putting random library classes/functions in this namespace
    template <typename T>
    class Optional
    {
    public:
        const static Optional<T> empty;
        static Optional<T> of(const T &v);
        static Optional<T> of(T *v);
        T get() const;
        bool hasValue() const;
        ~Optional() { if (value != nullptr) delete value; }
    private:
        Optional(T *v) : value{ v } {}
        const T *value;
    };

    template<typename T>
    const Optional<T> Optional<T>::empty = Optional::of(nullptr);

#include "optional.impl"
}

optional.impl

#include <stdexcept>

template<typename T>
Optional<T> Optional<T>::of(const T &t) {
    return Optional<T>{ new T(t) };
}

template<typename T>
Optional<T> Optional<T>::of(T *t) {
    if (t != nullptr) return Optional<T>{ new T(*t) };
    return Optional<T>{ nullptr };
}

template<typename T>
bool Optional<T>::hasValue() const {
    return value != nullptr;
}

template<typename T>
T Optional<T>::get() const {
    if (!hasValue()) throw std::runtime_error("Tried to get the value from an optional that has no value");
    return *value;
}

template<typename T>
bool operator==(const Optional<T>& t1, const Optional<T>& t2) {
    if (!t1.hasValue() && !t2.hasValue()) return true;
    else if (!t1.hasValue() || !t2.hasValue()) return false;
    return t1.get() == t2.get();
}

template<typename T>
bool operator!=(const Optional<T>& t1, const Optional<T>& t2) {
    return !(t1 == t2);
}

Rule of Three.

int main()
{
    extra::Optional<int> x = Optional<int>of(new int(5));
    extra::Optional<int> y(x);   // This still compilers.

    // But your problems don't appear until here
    // at the end of scope.
}

Design

Why do you force the construction of optional objects via the static methods?

    static Optional<T> of(const T &v);
    static Optional<T> of(T *v);

Why not allow people to just construct the objects in the normal way. It would seem more  natural to create objects of this type just like any other object!

Move semantics

You may not want copy semantics (since you are basically implementing a smart pointer. But you could do with implementing move semantics on your object.

Overall

I don't see the need.

If you want to return a pointer (but make sure it is managed). Then pass back a std::unique_ptr<T>.

If you are doing this as a learning exercise that's different. I'll give it a good going over once you have fixed the problems around the rule of three. But that is such a fundamental problem there is no point in reviewing this code further (Though in future version I would like a justification for the static of() methods and examples of why they are needed).


Adding very large arrays I am writing a stochastic simulation for a Yule process:

If you start with a certain number of bins each containing a random number of balls, you add another ball to an existing bin with a probability that is proportional to the number of balls within that bin, as well as create another bin with a single ball each time step.

Because I am trying to basically weed out any stochastic noise, I am using very large number of iterations, on the order of 1e6. This code was originally developed in MATLAB, making much less complicated, but is far too slow for large simulations.

Is there a better way of doing this?

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

void init_KISS();
unsigned int JKISS();
double uni_dblflt();
double array_sum();

int main()
{
    int t,count,i,j,k,ii,jj,kk;
    int desval;
    desval=1000000;
    unsigned int r;
    double v,u,bsum;
    init_KISS();
    for(i=0; i<100000;i++)
    {
        r = JKISS(); /* waste the first 100000 random numbers */
        uni_dblflt();
    }
    /*allocate memory for all params*/
    double *bins=malloc((desval+1)*sizeof(double));/*Urns, keeps track of actual "balls" in each bin*/
    if(bins==NULL)
    {
        printf("out of memory\n");
        exit(1);
    }

    double *p=malloc((desval+1)*sizeof(double));/* Urns/(sum of all "balls) i.e. the probability
                                                 that a new ball get placed in the ith urn. */
    if(p==NULL)
    {
        printf("out of memory\n");
        exit(1);
    }

    double *ptot=malloc((desval+1)*sizeof(double));/*Sum of all probabilities for the 1st to the ith
                                                    bin*/
    if(ptot==NULL)
    {
        printf("out of memory\n");
        exit(1);
    }

    for(i=0;i<(desval+1);i++)
    {
        bins[i]=0;
        ptot[i]=0;
        p[i]=0;
    }

    for(i=0;i<10;i++)/*seed 10 initial bins with 1 "ball"*/
    {
        v=1;
        bins[i]=v;
    }

    for(j=10;j<desval;j++)
    {
        bsum=array_sum(bins,desval);/*sum the balls in each bin*/
        for(k=0;k<j;k++) /* calculate the probability that with which a ball might be placed in
                          each urn*/
        {
            p[k]=bins[k]/bsum;
        }

        for(ii=0;ii<desval;ii++)/*Evaluate the probability total cutoffs*/
        {
            ptot[ii]=array_sum(p,ii);
        }
        u=uni_dblflt();
        count=0;
        while(u<=ptot[count])/*choose bin to place new ball. The numerical value of ptot(count)
                              is directly related to the probability space with which u can be
                              picked*/
        {
            count++;
        }

        bins[count]++;
        bins[j]=1;
    }

    for(jj=0;jj<desval;jj++)
    {
        printf("%.8f\n",bins[jj]);
    }

    free(bins);
    free(p);
    free(ptot);
    bins=NULL;
    p=NULL;
    ptot=NULL;
    return 0;
}

double array_sum(double *arr, int maxval)
{
    /* this function sums a large array*/
    int i;
    double sum;
    sum=0;

    for(i=0; i<maxval;i++)
    {
        sum +=arr[i];
    }

    return sum;
}

This is really an algorithms question. For example, consider this loop:

for(ii=0;ii<desval;ii++)/*Evaluate the probability total cutoffs*/
{
    ptot[ii]=array_sum(p,ii);
}

why are you calling array_sum() every time through this loop when the only thing that changes is the addition of one more element?
Why can't this loop be:

sum= 0;
ptot[0]= 0;
for(ii=1;ii<desval;ii++)/*Evaluate the probability total cutoffs*/
{
    ptot[ii]=(sum+= p[ii - 1]);
}


Android splash screen I have implemented a splash screen, to hold/pause the screen for few seconds and then launch the next screen. I am very enthusiastic to know if there is a more basic way to write the code.

public class SplashActivity extends BaseActivity {

    private final int SPLASH_TIME_OUT = 2000;

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_splash);

        Thread splashTimerThread = new Thread() {
            public void run() {
                try {
                    Thread.sleep(SPLASH_TIME_OUT);
                } catch (InterruptedException interruptedException) {
                    interruptedException.printStackTrace();
                } finally {
                    Intent intent = new Intent(SplashActivity.this, HomeActivity.class);
                    startActivity(intent);
                }
            }
        };
        splashTimerThread.start();
    }

    @Override
    protected void onPause() {
        super.onPause();
        finish();
    }
}

Using a thread for a SplashScreen is fine, however a much cleaner solution would to use a Handler as below...

public class SplashActivity extends BaseActivity {

    private final static int SPLASH_TIME_OUT = 2000;

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_splash);

        new Handler().postDelayed(new Runnable() {
            @Override
            public void run() {
                Intent intent = new Intent(SplashActivity.this, HomeActivity.class);
                startActivity(intent);
            }
        }, SPLASH_TIME_OUT);
    }

    @Override
    protected void onPause() {
        super.onPause();
        finish();
    }
}

Your SPLASH_TIME_OUT constant should be declared static.


Performance testing in PHP I need to test performance of a function in PHP. This has to happen accurately, of course. In the end I need to know how long it takes for a function to perform. What I'm doing now:

<?php
$start = microtime(true);
while (microtime(true) != $start + 1); // Wait a second to start up

$loops = 10;
$avg = 0;
for ($i = 0; $i < $loops; $i++) {
    $start = microtime(true);
    for ($c = 0; $c < 1000; $c++) {    // the 1000 may vary depending on the function
        theFunction();
    }
    $end = microtime(true);

    echo "Take $i: " . ($end-$start) . "s\n";

    $avg *= $i;
    $avg += $end - $start;
    $avg /= $i + 1;
}

echo "Average: " . $avg . "s\n";
?>

Is this a good way to test performance of a function in PHP? It returns about the same result all the time, within a margin of ~5% when the server does nothing else.

Let's assume the environment is clean (i.e. the server doesn't have to do anything else in the meanwhile), would this code be good for testing, or does it count too much or too little?

I don't see any immediate problems with it, but I would probably simplified it a bit. Not sure why you're doing it in two loops like that, but I might be missing something of course :)

<?php
$loops = 10000;

$start = microtime(true);

for ($i = 0; $i < $loops; $i++)
    theFunction();

$total = microtime(true) - $start;
$avg = $total / $loops;

echo "Average: " . $avg . "s\n";


Reverse binary representation of an int (only significant bits) This based on this question in StackOverflow. The accepted answer uses Convert.ToString(int, base) to get the binary string, reverses it and converts it back to int. Nice trick!

I have very little experience with bit twidling (using Flags is about it :p ) so I decided to try and code the solution to this problema without using Convert.

I came up with the following solution. I'd like to know easier ways to do this as there are probably many much better than the one I found.

public static IEnumerable<bool> ToBinary(this int n)
{
    for (int i = 0; i < 32; i++)
    {
        yield return (n & (1 << i)) != 0;
    }
}

public static int ToInt(this IEnumerable<bool> b)
{
    var n = 0;
    var counter = 0;

    foreach (var i in b.Trim().Take(32))
    {
        n = n | (i ? 1 : 0) << counter;
        counter++
    }

    return n;
}

private static IEnumerable<bool> Trim(this IEnumerable<bool> list)
{
    bool trim = true;

    foreach (var i in list)
    {
        if (i)
        {
            trim = false;
        }

        if (!trim)
        {
            yield return i;
        }
    }
}

And now you'd use it like this:

var reversed = n.ToBinary().Reverse().ToInt();

You could use while loop instead of the for loop.
Thus you don't need to trim zero bits, because it will continue to loop only while there is at least one significant bit in a value.
Another advantage of this approach is that the result can be calculated using integer arithmetics only without using any kind of collections.

static int Reverse(int input)
{
    uint x = unchecked((uint)input);
    uint y = 0;
    while (x != 0)
    {
        y <<= 1;    // Shift accumulated result left
        y |= x & 1; // Set the least significant bit if it is set in the input value
        x >>= 1;    // Shift input value right
    }
    return unchecked((int)y);
}

Usage example:

int t = 0x103;
Console.WriteLine(Convert.ToString(t, 2));
Console.WriteLine(Convert.ToString(Reverse(t), 2));

Result:

100000011
110000001