Converting Geographic Coordinates into Lambert2008 coordinates This is part of a game made in Unity3D.

I wrote this class to convert geographic coordinates into lambert 2008 coordinates based on this reference document:

http://www.ngi.be/Common/Lambert2008/Transformation_Geographic_Lambert_FR.pdf

Reason is that I'm using a Belgian National Geographic Institute map as background, and want to locate waypoint and player position with full accuracy.

Code works fine, and runs in .000001s but I'm unhappy with its readability.

I start by declaring all the constants I need

public const int a = 6378137;
public const double f = 1 / 298.257222101;
public const double lowerLatitude = 49.8333333 * Mathf.Deg2Rad;
public const double upperLatitude = 51.1666667 * Mathf.Deg2Rad;
public const double originLatitude = 50.797815 * Mathf.Deg2Rad;
public const double originLongitude = 4.359215833 * Mathf.Deg2Rad;
public const int originX = 649328;
public const int originY = 665262;

Then  I used properties for all values derived from those constants

public static double Excentricity {get => System.Math.Sqrt((2 * f) - (f * f));}

static double MLower { get => M(lowerLatitude);}
static double MUpper { get => M(upperLatitude);}

static double M(double latitude)
{
    return System.Math.Cos(latitude) / System.Math.Sqrt(1 - (Excentricity * Excentricity * System.Math.Pow(System.Math.Sin(latitude), 2)));
}

static double TLower { get => T(lowerLatitude); }
static double TUpper { get => T(upperLatitude); }
static double TOrigin { get => T(originLatitude); }

static double T(double latitude)
{

    return System.Math.Tan(Mathf.PI / 4 - latitude / 2) / System.Math.Pow((1 - Excentricity * System.Math.Sin(latitude)) / (1 + Excentricity * System.Math.Sin(latitude)), Excentricity / 2);
}

static double N { get => (System.Math.Log(MLower) - System.Math.Log(MUpper)) / (System.Math.Log(TLower) - System.Math.Log(TUpper)); }

static double G { get => MLower / (N * System.Math.Pow(TLower, N)); }

static double ROrigin { get => R(TOrigin); }

static double R (double t)
{
    return a * G * System.Math.Pow(t, N);
}

As you can see some of the calculations are written on very long lines. I'm unhappy with their readability, but I have no Idea on how to re-write them to make them look better while not messing up floating point approximations.

Finally I perform the actual conversion using all those calculated values.

public static Vector2 FromGeographicRelative(Vector2 coordinates)
{

    double t = T((double)coordinates.x * Mathf.Deg2Rad);
    double r = R(t);
    double angle = N * ((double)coordinates.y * Mathf.Deg2Rad - originLongitude);
    Vector2 lambertCoord = new Vector2
    {
        x = (float) (r * System.Math.Sin(angle)),
        y = (float) (ROrigin - r * System.Math.Cos(angle))
    };
    return lambertCoord;

}

Since the code is about implementing mathematical formulas I would make an exception and completely ignore the usual naming conventions. Instead, I would use the exact symbols as  that document.

This would mean a little bit copy/paste because it's difficult to write symbols like [ϕ, λ, θ] but the compiler can work with unicode.

I find that code like this:

var rad = 0.01745329252;

var ellipsoide = (a: 6378137, f: 1 / 298.257222101);
var ϕ1 = 49.8333333 * rad;
var ϕ2 = 51.1666667 * rad;
var ϕ0 = 50.797815 * rad;
var λ0 = 4.359215833 * rad;
var x0 = 649328;
var y0 = 665262;

is the most easy one to understand because you can compare it with the book almost without any translation.


Check if current time is within the timeframe using Joda-Time Novice Java developer here. I've never really used a time/date library before and I'm curious how an experienced developer would solve this. You're given 4 ints: startHour, startMinute, endHour and endMinute. Now, check if current time is within the given timeframe. Is there a more clean way of doing this than what I've done here:

private void checkTimeframe(int startHour, int startMinute, int endHour, int endMinute) {
    LocalDateTime now = LocalDateTime.now();
    LocalTime localTimeStart = new LocalTime(startHour, startMinute);
    LocalTime localTimeEnd = new LocalTime(endHour, endMinute);

    LocalDateTime startTime = new LocalDateTime(now.getYear(), now.getMonthOfYear(),
            now.getDayOfMonth(), startHour, startMinute);

    LocalDateTime endTime = new LocalDateTime(now.getYear(), now.getMonthOfYear(),
            now.getDayOfMonth(), endHour, endMinute);

    //Check if start/end is, for instance, 23:00 - 03:00
    if (localTimeStart.isAfter(localTimeEnd) || localTimeStart.equals(localTimeEnd)) {
        endTime = endTime.plusDays(1);
    }

    if ( (now.equals(startTime) || now.isAfter(startTime) ) && now.isBefore(endTime)) {
        System.out.println("Ok, we're within start/end");
    } else {
        System.out.println("Outside start/end");
    }
}

to me it looks like a simple check of number between range of numbers.
do I would do like this

turn hour and minute into one number that is hhmm to simplify the comparison
now its a simple check between range of numbers, taking into account the case of cross-date boundary

complete code:

private static void checkTimeframe(int startHour, int startMinute, int endHour, int endMinute) {
    // "concatanate" hour and minute into one number
    int startHourMinute = startHour * 100 + startMinute;
    int endHourMinute = endHour * 100 + endMinute;
    LocalDateTime now = LocalDateTime.now();
    int nowHourMinute = now.getHour() * 100 + now.getMinute();

    // if range within date - simple between boundaries check
    if (startHourMinute <= endHourMinute) {
        if (nowHourMinute >= startHourMinute && nowHourMinute <= endHourMinute) {
            System.out.println("Ok, we're within start/end");
        } else {
            System.out.println("Outside start/end");
        }
    // else (cross date boundary range) - check if now date is either within range of yesterday or within range tomorrow
    } else {
        if (nowHourMinute >= startHourMinute || nowHourMinute <= endHourMinute) {
            System.out.println("Ok, we're within start/end");
        } else {
            System.out.println("Outside start/end");
        }
    }
}


File loop and logic in controller of view header I have the following in the header.php file which is included in all of my views:

$dh = opendir(Vs.get_class($this).'/js') ;
while($script = readdir($dh)) {
    if(!is_dir($script))
    {
        echo '<script type="text/javascript" src="js/'.$script.'"></script>' ;
    }
}

$dh = opendir(Vs.get_class($this).'/css') ;
while($css = readdir($dh)) {
    if(!is_dir($css))
    {
        echo '<link type="text/css" href="css/'.$css.'" rel="stylesheet"/>' ;
    }
}

It's purpose is to autoload all the CSS and JS files for a particular view (which has the same name as the controller, hence get_class).

Should all this be a part of the associated controller or is how I have done it fine?

The way you are scanning the directory is going to get all files. If this is intentional thats cool, but if you are really just looking
for js & css files, use glob and scan for them directly. This will be much better in performance, and you can avoid the directory check.
It seems like its a brittle idea to have the paths mapped to a specific class name. There has to be an easier way to associate views
and view resources rather than referencing $this.
This does not belong in the controller unless you are assigning the view data there. In frameworks like Zend, there are action helpers
which take care of this for you. If you don't have access to that, create a base controller class and do a magic helper method to keep the
functionality the same across all pages. This will allow you to maintain this functionality closer, and you can still use the $this if needed.
Is this really a good idea to just scan a directory and include all files? What happens if a new developer comes along and dumps a js file
they were testing something in, in the directory and it got included by accident? If there are very few includes, try to just manually put them in;
either through a layout or just hardcoding them in there. If you really have that much js and css being included in.... maybe there is a more
fundamental problem.

Better Scandir w/ Glob:

$jsGlob = Vs.get_class($this).'/js/*.js';
foreach (glob($jsGlob) as $js)
{
    echo '<script type="text/javascript" src="$js"></script>' ;
}

$cssGlob = Vs.get_class($this).'/css/*.css';
foreach (glob($cssGlob) as $css)
{
    echo '<link type="text/css" href="$css" rel="stylesheet"/>' ;
}


Filtering multiple request parameters This is pretty straightforward, it takes requests from the frontend, checking if it is present on the request, if it does, an action is taken for that if statement.
I wonder if there is a way to remove some if statements or make it less complicated.
private function applyFiltering($query, $request)
{
    if ($request->has('active_group')) {
        $query->whereNotIn('status_id', [1, 6, 4]);
        $query->whereHas('userStatus', function ($query) use ($request) {
            $query->where('user_id', $request->user()->id)->whereNotIn('status_id', [1, 6, 4]);
        });
    }

    if ($request->has('waiting_group')) {
       $query->whereNotIn('status_id', [1, 6]);
        $query->whereHas('userStatus', function ($query) use ($request) {
              $query->where('user_id', $request->user()->id)->whereNotIn('status_id', [1, 6])->where('status_id',4);
         });
    }

    if ($request->has('meeting_group')) {
        $query->where('type_id', 2);
    }

    if ($request->has('comp_meeting_group')) {
        $query->where('type_id', 4);
    }

    if ($request->has('completed_group')) {
            $query->whereHas('userStatus', function ($query) use ($request) {
                $query->where('user_id', $request->user()->id)->where('status_id', 1);
            });
    }

    if ($request->has('canceled_group')) {
        $query->whereHas('userStatus', function ($query) use ($request) {
            $query->where('user_id', $request->user()->id)->where('status_id', 6);
        });
    }

    if ($request->has('shift_group')) {
        $query->where('status_id', 1)->where('type_id', 6);
    }

    if ($request->has('announce_group')) {
        $query->where('type_id', 3);
    }

    if ($request->has('task_id')) {
        $query->where('id', 'like', '%' . $request->input('task_id') . '%');
    }

    if ($request->has('task_statuses')) {
        $statuses = $request->input('task_statuses');
        $query->whereIn('status_id', $statuses);
    }
    if ($request->has('task_types')) {
        $statuses = $request->input('task_types');
        $query->whereIn('type_id', $statuses);
    }
    if ($request->has('task_priorities')) {
        $statuses = $request->input('task_priorities');
        $query->whereIn('priority_id', $statuses);
    }
    if ($request->has('task_invite_users')) {
        $toUsers = $request->input('task_invite_users');
        $query->whereHas('taskAssignment', function ($query) use ($request, $toUsers) {
            $query->whereIn('assigned_to', $toUsers);
        });

        // $query->whereIn('to_user_id',$statuses);
    }

    if ($request->has('task_users')) {
        $byUsers = $request->input('task_users');
        $query->whereHas('taskAssignment', function ($query) use ($request, $byUsers) {
            $query->whereIn('assigned_by', $byUsers);
        });
    }
}

You could try something like this:

private function applyFiltering($query, $request)
{

    $query->when($request->has('active_group'), function($query) use($request) {
        $query->whereNotIn('status_id', [1, 6, 4]);
        $query->whereHas('userStatus', function ($query) use ($request) {
            $query->where('user_id', $request->user()->id)->whereNotIn('status_id', [1, 6, 4]);
        });
    })->when($request->has('waiting_group'), function($query) use ($request) {
        $query->whereNotIn('status_id', [1, 6]);
        $query->whereHas('userStatus', function ($query) use ($request) {
              $query->where('user_id', $request->user()->id)->whereNotIn('status_id', [1, 6])->where('status_id',4);
         });
    })
    ........
    ........
    ........
}
```


Game prototype with methods to manage players I don't really ever write JS like this in my real workflow, but I was just trying to answer a question for someone - that was then deleted.

I wrote the following and I'd like to know if this is close to smart / or how I can improve on these patterns. I'm interested in prototypes but also functional approaches. I am pretty familiar with es2015 - so if you please, that isn't the kinda of thing I'm trying to learn in this case - so please stick with 'var' etc.

jsFiddle

"use strict";

console.clear(); // just to keep things clean in the console... not for production

function Game(newGameTitle) {
  this.title = newGameTitle;
  this.players = [];
  this.createPlayer = createPlayer;
  this.findPlayer = findPlayer;
  this.listPlayers = listPlayers;
  console.log('Game "', newGameTitle, '" created.');
}

function createPlayer(newPlayerName) { // assigned on line 8
  console.log('player', newPlayerName, 'created.');
  // get this player into the array somehow...
  var newPlayerObject = {
    name: newPlayerName,
    level: 1, // reasonable default?
  };
  // you could use a constructor here like I did with game to create a prototype or something too
  // then you can have Player.methods to 'level up' etc. - but the point should be made... you'd use the new keyword here in that case
  this.players.push(newPlayerObject);
  // 'this' is going to refer to the object that eventually 'calls' this... so the 'game' hopScotch
  // same goes for all it's uses
  console.log(this); // show the changes to the game
  return this; // in case you want to chain? not sure...
}

function findPlayer(playerName) { // assigned on line 9
  console.log('found', playerName);
  function isPlayer(targetName) {
    return targetName = name;
  }
  return this.players.find(isPlayer);
}

function listPlayers() { // assigned on line 10
  console.log('Player list:');
  this.players.forEach( function(currentPlayer, currentIndexInArray, fullArray /*if needed*/) {
    console.log(currentIndexInArray, currentPlayer);
  });
  return this;
  // I guess this doesn't have any side-effects - but what if I wanted to chain it?
}

// create 'game'
var hopScotch = new Game('Hopscotch');
console.log(hopScotch);

// add some players
hopScotch.createPlayer('@sheriffderek');
hopScotch.createPlayer('@ivy');
hopScotch.createPlayer('@valentineRose');

// try some methods
hopScotch.findPlayer('@sheriffderek');
hopScotch.listPlayers();

// checking chaining
hopScotch.createPlayer('@h2whoah').findPlayer('@h2whoah');
hopScotch.listPlayers().findPlayer('@ivy');

Bug

In this code, what is the return targetName = name line trying to achieve?

function findPlayer(playerName) { // assigned on line 9
  console.log('found', playerName);
  function isPlayer(targetName) {
    return targetName = name;
  }
  return this.players.find(isPlayer);
}

name is undefined, and probably you wanted to use == instead of =.

Also, the function should not print found ...,
before actually finding anything.

Code organization

The functions that should be called on Game instances should not be in the global namespace.
Not only they pollute the global namespace,
they are not useful and misleading without a Game instance.

Also, unless you want to allow manipulating the players array directly,
it would be better to hide it from users.

Printing inside functions is a bad practice unless it is their dedicated purpose.

This would be a better organization:

function createGame(title) {
  var players = [];

  function addPlayer(name) {
    players.push({ name: name });
  }

  function findPlayer(name) {
    return players.filter(function(player) { return player.name === name; });
  }

  function printPlayers() {
    console.log('Player list:');
    players.forEach(function(player, index) {
      console.log(index, player);
    });
  }

  return {
    title: title,
    addPlayer: addPlayer,
    findPlayer: findPlayer,
    printPlayers: printPlayers
  };
}

"just-in-case" programming

You made it possible to chain calls to createPlayer, along with this comment:

return this; // in case you want to chain? not sure...

When you're not sure to implement something,
there's a really simple rule of thumb you can follow: don't do it.
Everything in the code should have a purpose,
avoid writing code "just in case it might be useful someday".


Remove known Excel passwords with PowerShell I have this PowerShell code that loops through Excel files in a specified directory; references a list of known passwords to find the correct one; and then opens, decrypts, and saves that file to a new directory. This currently processes ~40 workbooks in roughly 5 minutes, which is still longer than I'd like (but better than the initial 40 minute runtime).

Note that I previously posted under the same title here, but now I am looking for a review after solving a rather obvious error.

# Get Current EXCEL Process ID's so they are not affected but the scripts cleanup
# SilentlyContinue in case there are no active Excels
$currentExcelProcessIDs = (Get-Process excel -ErrorAction SilentlyContinue).Id

$a = Get-Date

$ErrorActionPreference = "SilentlyContinue"

CLS

# Paths
$encrypted_path = "C:\PoShTest\Encrypted"
$decrypted_Path = "C:\PoShTest\Decrypted\"
$processed_Path = "C:\PoShTest\Processed\"
$password_Path  = "C:\PoShTest\Passwords\Passwords.txt"

# Load Password Cache
$arrPasswords = Get-Content -Path $password_Path

# Load File List
$arrFiles = Get-ChildItem $encrypted_path

# Create counter to display progress
[int] $count = ($arrfiles.count -1)

# New Excel Object
$ExcelObj = $null
$ExcelObj = New-Object -ComObject Excel.Application
$ExcelObj.Visible = $false

# Loop through each file
$arrFiles| % {
    $file  = get-item -path $_.fullname
    # Display current file
    write-host "`n Processing" $file.name -f "DarkYellow"
    write-host "`n Items remaining: " $count `n

    # Excel xlsx
    if ($file.Extension -like "*.xls*") {

    # Loop through password cache
        $arrPasswords | % {
            $passwd = $_

            # Attempt to open file
            $Workbook = $ExcelObj.Workbooks.Open($file.fullname,1,$false,5,$passwd)
            $Workbook.Activate()

            # if password is correct save decrypted file to $decrypted_Path
                if ($Workbook.Worksheets.count -ne 0 )

                {
                    $Workbook.Password=$null
                    $savePath = $decrypted_Path+$file.Name
                    write-host "Decrypted: " $file.Name -f "DarkGreen"
                    $Workbook.SaveAs($savePath)

             # Added to keep Excel process memory utilization in check
                    $ExcelObj.Workbooks.close()

             # Move original file to $processed_Path
                    move-item $file.fullname -Destination $processed_Path -Force

                }
                else {
            # Close Document
                    $ExcelObj.Workbooks.Close()
                }
        }

    }

$count--
# Next File
}
# Close Document and Application
    $ExcelObj.Workbooks.close()
    $ExcelObj.Application.Quit()

Write-host "`nProcessing Complete!" -f "Green"
Write-host "`nTime Started   : " $a.ToShortTimeString()
Write-host "Time Completed : " $(Get-Date).ToShortTimeString()
Write-host "`nTotal Duration : "
NEW-TIMESPAN –Start $a –End $(Get-Date)

# Remove any stale Excel processes created by this scripts execution
Get-Process excel -ErrorAction SilentlyContinue | Where-Object{$currentExcelProcessIDs -notcontains $_.id} | Stop-Process

Once you've found the right password and moved the file, then there is no need to try the rest of the passwords on it. You can break out of the loop with a break command.

(You have to change the pipeline % into a foreach ( in ) because otherwise the break would break out of the outer pipeline too. Thanks to Lieven in the comments for that.)

foreach ($passwd in $arrPasswords)
{
    # ...

    # if password is correct save decrypted file to $decrypted_Path
    if ($Workbook.Worksheets.count -ne 0 )
    {
        # ...
        break;
    }

    # ...
}

That may or may not speed things up.

Otherwise, I haven't tried them but I understand that there are .NET Excel libraries that you can use to manipulate Excel files without having to use Excel itself. COM automation is pretty slow, so using one of these libraries might be faster.


Predicate class implementation in TypeScript I am have created a predicate builder that takes predicates (functions with a single input and produce a boolean) and can chain them using boolean logic, so AND, OR and NOT. It is inspired by the Java Predicate class. This is mostly to practice my TypeScript, so general feedback is of course appreciated. However, I am interested in few points:

Naming. I don't like the Builder suffix here as it doesn't seem to fit. Although, I cannot exactly explain why it feels wrong. Partly, I'm used to builders producing an object as a result, not to be containers for the logic that will be later applied. This seems more of a Functor. Furthermore "predicate" is the mathematical and general term for a function of type (x: any) => boolean, so I don't want to name the class that way. Java gets away with it because it has functional interfaces.
Repetition. When using .and() and .or() the return is the same aside from the operator being used. This feels off. When I add a new method like .xor(), then on one hand, I don't want to repeat this. On the other, .xor() will not be implemented using a single operator. There should be a way to generalise the logic that I simply don't see.
Overloaded methods. Since you can't actually have method overloading in TypeScript/JavaScript you need to accept all possible signatures and sort out what to do in the method. Is .extractPredicate() a decent way to go about this in my case? Or is there something more idiomatic or clearer?

interface Predicate<T> {
  (x: T) : boolean
}

class PredicateBuilder<T> {
  condition: Predicate<T>;

  constructor(condition: Predicate<T>) {
    this.condition = condition;
  }

  public and(input: PredicateBuilder<T> | Predicate<T>): PredicateBuilder<T> {
    const p: Predicate<T> = this.condition;
    const q: Predicate<T> = this.extractCondition(input);

    return new PredicateBuilder((x: T) => p(x) && q(x));
  }

  public or(input: PredicateBuilder<T> | Predicate<T>): PredicateBuilder<T> {
    const p: Predicate<T> = this.condition;
    const q: Predicate<T> = this.extractCondition(input);

    return new PredicateBuilder((x: T) => p(x) || q(x));
  }

  public not(): PredicateBuilder<T> {
    return new PredicateBuilder((x: T) => !this.condition(x));
  }

  public apply(x: T): boolean {
    return this.condition(x);
  }

  private extractCondition(input: PredicateBuilder<T> | Predicate<T>): Predicate<T> {
    let condition: Predicate<T>;
    if ("condition" in input) {
      condition = input.condition;
    } else {
      condition = input;
    }

    return condition;
  }
}

Playground Link including a small set of unit tests

As pointed out as a comment, I was unhappy about the extractCondition function.
This function handles, variables which do conform your Predicate<T> type. Which is fine, but I think this doesn't belong here.
Because a Predicate should have an accept or, if we talk about the Java implementation, test function.
This is not given when we can pass on any random function which do return a boolean. But does not conform our design of a Predicate.

Next naming, the PredicateBuilder seems okay-ish, but it feels wrong. As it isn't a builder in the classical sense.
Furthermore we have to work around functional interfaces, and this is I think the main pain point. To achieve this I would propose to rename the Predicate<T> interface, as it should be describing what a Predicate holds (a function which takes a parameter and returns a boolean). Therefore the name Predicate is available again and should be used instead of PredicateBuilder.

The repetition regarding and, or, etc... feels also some kind off. But I think that's because you always do extractCondition and other stuff (I've eliminated this in my suggestion, as the Java Predicate implementation does basically the same). Plus I've create a static method Predicate#of(...) to easier "get" a new Predicate. This reduces the mess a bit, but changing the operands is also still a bit clumsy, in my opinion.

Oh and btw. I've felt free to use arrow functions :-)

interface InternalPredicate<T> {
  (x: T) : boolean
}

class Predicate<T> {
  constructor(private condition: InternalPredicate<T>) {}

  public static of = <T>(condition: InternalPredicate<T>) => {
    return new Predicate(condition);
  }

  public and = (input: Predicate<T>): Predicate<T> =>
    Predicate.of((x: T) => this.apply(x) && input.apply(x));

  public or = (input: Predicate<T>): Predicate<T> =>
    Predicate.of((x: T) => this.apply(x) || input.apply(x));

  public not = (): Predicate<T> =>
    Predicate.of((x: T) => !this.apply(x));

  public apply = (x: T): boolean => this.condition(x);
}

Those are my two cents to the Predicate implementation, feel free to discuss them with me.

Alternative solution to eliminate the InternalPredicate and to support functions which are booleans:

type PredicateType<T> = (x: T) => boolean;

class Predicate<T> {
        constructor(private condition: PredicateType<T>) {}

        private static isInstance = <T>(input: Predicate<T> | PredicateType<T>): Predicate<T> => (input instanceof Predicate) ? input : Predicate.of(input);

        public static of = <T>(condition: PredicateType<T>) => new Predicate(condition);

        public and = (input: Predicate<T> | PredicateType<T>): Predicate<T> =>
          Predicate.of((x: T) => this.apply(x) && Predicate.isInstance(input).apply(x));

        public or = (input: Predicate<T> | PredicateType<T>): Predicate<T> =>
          Predicate.of((x: T) => this.apply(x) || Predicate.isInstance(input).apply(x));

        public not = (): Predicate<T> =>
          Predicate.of((x: T) => !this.apply(x));

        public apply = (x: T): boolean => this.condition(x);
}

It comes pretty close the the extractCondition approach, but the main difference here is, that a new instance is created if the input is a raw function. This adds up to be more robust to changes and being less error prone. Another advantage is, we don't care about how the predicate internally works, we're just in charge of returning a predicate based on the input. This clears up the use case of the function a bit more.

EDIT: As discussed in the comments, it is nicer to merge the private isInstance function into the static of function.


Comparing IDs in two arrays I have two arrays that I need to compare if their IDs are equal. This is how I am currently doing it:

PFQuery *earnedQuery = [PFQuery queryWithClassName:@"EarnedAchievement"];
[earnedQuery whereKey:@"user" equalTo:[PFUser currentUser]];
[earnedQuery findObjectsInBackgroundWithBlock:^(NSArray *objects, NSError *error) {

    for (Achievement *achievement in allAchievements) {

        for (PFObject *earnedAchievement in objects) {

            if ([achievement.id isEqualToString:earnedAchievement[@"achievmentId"]]) {

                //user has earned this achievement
                achievement.earned = [NSNumber numberWithBool:YES];
                achievement.earnedDate = earnedAchievement[@"earnedOn"];

            }

        }

    }

    [context save:&error];

}];

Is there a better or more efficient way to do this outside of the two for in loops?

The first thing we can do to improve the efficiency of this code is to remember to check for an error and make sure that the returned objects array returns anything before we do anything.  Generally, code from Apple libraries would guarantee our objects array to be empty if there were any sort of error, but some libraries don't make that guarantee.

We can also store our allAchievements locally as an NSDictionary rather than an array.  If allAchievements is an NSDictionary, our code looks something more like this:

[earnedQuery findObjectsInBackgroundWithBlock:^(NSArray *objects, NSError *error) {
    for (PFObject *earnedAchievement in objects) {
        Achievement *achievement = allAchievements[earnedAchievement[@"achievmentId"]];
        if (achievement) {
            achievement.earned = @YES;
            achievement.earnedDate = earnedAchievement[@"earnedOn"];
        }
    }
    [context save:&error];
}];

Now we eliminate the nested loop.  And we only run through the outer loop once per earnedAchievement (which may be zero times).

Also, keep in mind that everything within the findObjectsInBackgroundWithBlock: is executing in the background.

Some other comments...

I would eliminate the use of magic strings.  Define all of your keys as constants somewhere.  This serves multiple purposes.

If we ever decide to actually change the key, it's changed everywhere all at once.
Typing magic strings is prone to typos, where as using a variable won't compile if we've made a typo (unless we typo into another variable name).
Typing magic strings won't auto-complete (which makes typos more likely, see point 2), but constant variables will.


Decorator Design Pattern in c++ I wrote a version of decorator pattern in c++ as I wasn't able to find any good source of decorator pattern implementation in C++. I want to know if I implemented it correctly. Here is the code-

//implementing the pizza example in c++

#include "bits/stdc++.h"
using namespace std;

class Pizza{
protected:
    string description = "";
    int cost = 0;
public:
    string getdescription(){
        return description;
    }
    int getcost(){
        return cost;
    }
};
//pizzas
class PeppyPaneer: public Pizza{
public:
    PeppyPaneer(){
        cost = 100;
        description = description + " PeppyPaneer";
    }
};
class Magharita: public Pizza{
public:
    Magharita(){
        cost = 200;
        description = description + " Magharita";
    }
};

//toppings
class Barbeque: public Pizza{
public:
    Barbeque(Pizza *pizza){
        cost = pizza->getcost() + 50;
        description = pizza->getdescription()+ " Barbeque";
    }
};

class Paneer: public Pizza{
public:
    Paneer(Pizza *pizza){
        cost = pizza->getcost() + 60;
        description = pizza->getdescription() + " Paneer";
    }
};

int main(){

    Pizza *pizza = new PeppyPaneer();
    cout << pizza->getcost() << " " << pizza->getdescription() << "\n";

    pizza = new Barbeque(pizza);
    cout << pizza->getcost() << " " << pizza->getdescription() << "\n";

    pizza = new Paneer(pizza);
    cout << pizza->getcost() << " " << pizza->getdescription() << "\n";

    return 0;
}

Here are some comments on your code and design.

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit.

Use the required #includes

The code uses std::string which means that it should #include <string>.  It also needs #include <iostream>.

Don't #include headers that aren't needed

The inverse of the advice above is to not include header files that are not needed.  In this case, "bits/stdc++.h" should be removed.  It's not standard and will only increase compilation times, so you should probably avoid it in favor of using the standard #includes as actually needed.

Don't leak memory

Each time your code in main creates a new pizza, it loses any pointer to the old one, making it a certainty that memory is leaked.  There are two ways to deal with that.  One is to use delete to match each new.  Another alternative is to use smart pointers instead.

C++ isn't Java

Code like this looks more like Java than idiomatic C++:

std::string getdescription(){
    return description;
}
int getcost(){
    return cost;
}

Instead of writing generic getters and setters in the Java style, write idiomatic C++ instead:

std::string desc() const {
    return m_description;
}
int cost() const {
    return m_cost;
}

Here I've used a relatively common idiom of prefixing class variables with m_ and omitting the word get.  Additionally, both functions are declared const because they do not alter the underlying object.

Reread the decorator pattern description

I don't think your code actually faithfully implements the decorator pattern. Consider instead a revision to your pizza model in which a new pizza retains a reference to the underlying basic pizza model passed to the constructor.  That would be a better potential illustration for the use of a decorator pattern.  See https://en.wikipedia.org/wiki/Decorator_pattern for more details.


Base N To Decimal Converter Problem

Implement a base N to base 10 (decimal) converter, where N is between 2 and 36 inclusive.

Implementation

Am I over-complicating things in calculateDecimalCharacterValue?

public class BaseToDecimalConverter {

  public static int convertToDecimal(int base, String input) {
    if (base < 2) {
      throw new IllegalArgumentException();
    }

    if (base > 36) {
      throw new IllegalArgumentException();
    }

    if (input == null) {
      throw new IllegalArgumentException();
    }

    int length = input.length();
    int sum = 0;
    for (int i = 0; i < length; i++) {
      int digitValue = calculateDecimalCharacterValue(input.charAt(i), base);
      sum += digitValue * Math.pow(base, length - 1 - i);
    }

    return sum;
  }

  public static int calculateDecimalCharacterValue(char c, int base) {
    if (base < 2) {
      throw new IllegalArgumentException();
    }

    if (base > 36) {
      throw new IllegalArgumentException();
    }

    if (Character.isDigit(c)) {
      int numericValue = Character.getNumericValue(c);
      if (numericValue < 0) {
        throw new IllegalArgumentException();
      }
      return numericValue;
    }

    if (Character.isAlphabetic(c) && base > 10) {
      int alphabetValue = Character.toUpperCase(c) - 'A' + 10;
      if (alphabetValue > base) {
        throw new IllegalArgumentException();
      }
      return alphabetValue;
    }

    throw new IllegalArgumentException();
  }
}

It's weird that you offer two public functions: a general-purpose convertToDecimal(), and a calculateDecimalCharacterValue() that does the same thing but with more restrictive validation.  The latter should be a private helper function.

You are over-complicating things by a lot: the whole thing could be accomplished with Integer.parseInt(s, radix).  And — not that you would need it — calculateDecimalCharacterValue(c, base) could be done using Character.digit(ch, radix).

Technically, you aren't converting to and from decimal (base ten).  You are parsing numbers in a base-N representation to an int; it just happens that ints are conventionally formatted in base ten when you do things like System.out.println().  So, the function names aren't quite right.

You shouldn't need to use Math.pow().  The usual algorithm goes like this:

public static int convertFromBase(int base, String input) {
    // Validation omitted here...

    int result = 0;
    for (char c : input.toCharArray()) {
        result = result * base + Character.digit(c, base);
    }
    return result;
}


Find the 2 distinct highest values in an array I want to know if there is a way to improve my code.  It finds the two highest values in an array, and these numbers need to be distinct.

I don't want to sort the values; I just want to find them.

My idea is to create a new array if the length is even and compare the pair of values to find the minimum and maximum values.

So, later, I find the second highest value.

package doze;

public class Elementos {

    public int maxValue(int array[], int arrayLength) {

        arrayLength = array.length;

        if ((arrayLength % 2) > 0) {
            arrayLength++;
            int aux[] = array;
            array = new int[arrayLength];
            for (int i = 0; i < aux.length; i++) {
                array[i] = aux[i];
            }
            array[(arrayLength - 1)] = aux[(arrayLength - 2)];
        }

        int maxValue = array[0];
        int minValue = array[0];
        int i = 0;

        while (i != arrayLength) {
            if (array[i] > array[i + 1]) {
                if (array[i] > maxValue) {
                    maxValue = array[i];
                }
            if (array[i + 1] < minValue) {
                minValue = array[i + 1];
            }
            } else {
                if (array[i + 1] > maxValue) {
                    maxValue = array[i + 1];
                }
                if (array[i] < minValue) {
                    minValue = array[i];
                }
            }
            i += 2;
        }

        int secondMaxValue = minValue;
        i = 0;
        while (i != arrayLength) {
            if ((array[i] > minValue) && (array[i] < maxValue)) {
                minValue = array[i];
                secondMaxValue = minValue;
            }
            i += 1;
        }

        return maxValue;
    }
}

Here's a simple implementation which will return the max (first index in return array) and second distinct max (second index in return array) value as an array.  If the list is size zero or there is no distinct max value, then Integer.MIN_VALUE is returned.

/**
 * @param integer array
 * @return an array comprising the highest distinct value (index 0) and second highest distinct
 *         value (index 1), or Integer.MIN_VALUE if there is no value.
 */
public int[] findTwoHighestDistinctValues(int[] array)
{
    int max = Integer.MIN_VALUE;
    int secondMax = Integer.MIN_VALUE;

    for (int value:array)
    {
        if (value > max)
        {
            secondMax = max;
            max = value;
        }
        else if (value > secondMax && value < max)
        {
            secondMax = value;
        }
    }
    return new int[] { max, secondMax };
}


Extracting min and max salary from string What I want is to do is to extract min/max range of salary from a text which contains either hourly or annual salary.
import re

# either of the following inputs should work
input1 = "$80,000 - $90,000 per annum"
input2 = "$20 - $24.99 per hour"

salary_text = re.findall("[\$0-9,\. ]*-[\$0-9,\. ]*", input1)

if salary_text and salary_text[0]:
    range_list = re.split("-", salary_text[0])

    if range_list and len(range_list) == 2:
        low = range_list[0].strip(' $').replace(',', '')
        high = range_list[1].strip(' $').replace(',', '')

````

The , commas are a nice twist.
I feel you are stripping them a bit late,
as they don't really contribute to the desired solution.
Better to lose them from the get go.
Calling .findall seems to be overkill for your problem specification --
likely .search would suffice.

salary_text = re.findall("[\$0-9,\. ]*-[\$0-9,\. ]*", input1)

The dollar signs, similarly, do not contribute to the solution,
your regex could probably just ignore them if your inputs are fairly sane.
Or even scan for lines starting with $ dollar, and then the regex ignores them.

range_list = re.split("-", salary_text[0])

There is no need for this .split -- the regex could have done this for you already.
Here is what I recommend:
def find_range(text):
    if text.startswith('$'):
        m = re.search(r'([\d\.]+) *- *\$?([\d\.]+)', text.replace(",", ""))
        if m:
            lo, hi = m.groups()
            return float(lo), float(hi)
    return None, None

print(find_range('$80,000 - $90,000 per annum'))
print(find_range('$20 - $24.99 per hour'))

The raw string regex r'[\d\.]+' picks out one or more numeric characters,
which can include decimal point.
And putting ( ) parens around a regex makes it a capturing group --
we have two such groups here.
Finally,  *- *\$? lets us skip a single - dash with optional whitespace
and at most one optional $ dollar sign.


Monitor asynchronous tasks, tracking their running times Scenario: I'm getting requests through a (thread-safe) queue. Each request then needs to be handled in a separate thread. There is a chance that the function (which is actually calling a Java-program via popen, and polling its output) takes a very long time.  From the main thread I need a mechanism to indicate that situation (basically, measuring thread-running-time).

In my example I'm trying to 'enrich' std::future with some time information. The sample runs seamlessly - but I'm uncertain if this is the correct way.

Here is a very simple demo that mimics what I'm trying to achieve (where ThreadFunc stands in for my real processing code):

#include <iostream>

#include <thread>
#include <future>
#include <chrono>
#include <vector>
#include <random>

typedef std::future<int> FutureResultInt;

int ThreadFunc( )
{
  std::random_device rd;
  std::mt19937 mt(rd());

  const int iRand = std::uniform_int_distribution<int>(2000, 6000)(mt);

  std::cout << "ThreadFunc waiting for [" << iRand << "] ms ... " << std::endl;

  std::this_thread::sleep_for(std::chrono::milliseconds(iRand));

  std::cout << "ThreadFunc [" << iRand << "] done" << std::endl;

  return iRand;
}

class CFutureTest
{
public:
  CFutureTest() = delete;

  CFutureTest(FutureResultInt&& fr)
    : m_start(std::chrono::system_clock::now())
    , m_result()
  {
    m_result = std::move(fr);
  };

  int GetAge() const
  {
    return std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::system_clock::now() - m_start).count();
  }

// private:
  FutureResultInt m_result;
  std::chrono::time_point<std::chrono::system_clock> m_start;
};

int main()
{
  std::vector< CFutureTest > futures;
  for (int i = 0; i < 5; i++)
    futures.push_back(std::move(std::async(std::launch::async, ThreadFunc)));

  while (futures.size() > 0)
  {
    for (std::vector< CFutureTest >::iterator it = futures.begin(); it != futures.end(); ++it)
    {
      CFutureTest& future = *it;

      const std::future_status stat = future.m_result.wait_for(std::chrono::milliseconds(1));

      switch (stat)
      {
      case std::future_status::timeout:
        if (future.GetAge() > 4000)
        {
            std::cout << "Thread has exceeded the time limit" << std::endl;
        }
        continue;

      case std::future_status::deferred:
        std::cout << "std::future_status::deferred" << std::endl;
        continue;
      }

      const int iResult = future.m_result.get();

      std::cout << "future returned [" << iResult << "] (removing!)" << std::endl;
      futures.erase(it);

      if (futures.size() < 1)
        break;

      it = futures.begin();
    }
  }
  return 0;
}

Ugly typedef

I'm not a big fan of this:

typedef std::future<int> FutureResultInt;

It's not significantly shorter or easier to read, it doesn't isolate the user from an underlying type, and it only serves to slow down my reading every time I hit it.  That's somewhat subjective, of course, but I don't believe it adds value.  I'd be happier with it if it told us what it's for, rather than merely what it is.  I'm thinking of something like:

using TimerTaskResult = std::future<int>;

Now the name conveys something that's not just duplicating the standard name.

Member initialization

My compiler warns me that the initializer list is in a misleading order:

203856.cpp:47:54: warning: ‘CFutureTest::m_start’ will be initialized after [-Wreorder]
203856.cpp:46:19: warning:   ‘FutureResultInt CFutureTest::m_result’ [-Wreorder]
203856.cpp:33:3: warning:   when initialized here [-Wreorder]
   CFutureTest(FutureResultInt&& fr)
   ^~~~~~~~~~~

I'd also recommend value-initializing m_result rather than default-constructing followed by move-assigning:

  CFutureTest(FutureResultInt&& fr)
      : m_result{std::move(fr)}
      , m_start{std::chrono::system_clock::now()}
  {
  }

Missing case

203856.cpp: In function ‘int main()’:
203856.cpp:63:14: warning: enumeration value ‘ready’ not handled in switch [-Wswitch]
       switch (stat)
              ^

I like to have this warning enabled.  We could provide a minimal case std::future_status::ready: break;, or we could unify the control flow by changing the other branches from continue to break and inlining the following code into the ready case like this:

        switch (stat) {
        case std::future_status::timeout:
            if (future.GetAge() > 4000) {
                std::cout << "Thread has exceeded the time limit" << std::endl;
            }
            break;

        case std::future_status::deferred:
            std::cout << "std::future_status::deferred" << std::endl;
            break;

        case std::future_status::ready:
            {
                const int iResult = future.m_result.get();
                std::cout << "future returned [" << iResult << "] (removing!)" << std::endl;
                futures.erase(it);

                if (futures.empty()) {
                    return 0;
                } else {
                    it = futures.begin();
                }
            }
        }

Instead of returning to futures.begin() when we harvest a result, it's arguably better to keep going from the next element if there is one:

                it = futures.erase(it);
                if (it == futures.end()) {
                    if (futures.empty()) {
                        return 0;
                    } else {
                        it = futures.begin();
                    }
                }

Interleaved output

I know it's not part of your production code, but it's quite irritating to have the different threads' output interrupting each other mid-line.  We can provide a class to hold a lock while several items are written to a stream:

class LogStream
{
    static std::mutex mutex;

    std::lock_guard<std::mutex> guard{mutex};
    std::ostream& stream{std::clog};

public:
    LogStream() {}
    template<typename T>
    std::ostream& operator<<(T&& t) { return stream << std::forward<T>(t); }
};

std::mutex LogStream::mutex;

And use it like this:

LogStream() << "ThreadFunc waiting for [" << iRand << "] ms ... " << std::endl;

Be clear about units

I don't like this function name:

int GetAge() const

Perhaps change it to getAgeMillis(), or how about making it a template method? Like this:

template<typename TimeUnit = std::chrono::seconds>
int GetAge() const
{
    auto now = std::chrono::system_clock::now();
    return std::chrono::duration_cast<TimeUnit>(now - m_start).count();
}

That also helps us deal with the very long lines that are a perennial problem with duration_cast.

Avoid polling

This is the big one.  It's very power-inefficient to poll every 1ms to see whether you have any results; that's a bad thing on battery-powered systems, but it's just as important in a server farm.

What we need to do instead is have each finishing thread notify a condition variable.  When we're woken, we then examine all the futures, picking up any where is_ready().

Unfortunately, that means we can't use std::async(), as its future isn't made ready until the called function has returned.  We'll need to implement our own version that can accept a std::condition_variable& to be notified after it sets the future's value.

You might be able to pick up some advice from Stack Overflow - here's a couple of starting points that might have something relevant:

Signaling main thread when std::future is ready to be retrieved
How to implement std::when_any without polling?


arrayHistogram implementation I wrote this code, called arrayHistogram, that would generate a random number within a set range, and print it in an array randomIntArray. It would then make another array arrayHist that would count how many times each number in randomIntArray occurred. The program works, but, I feel I have a few semantic errors in the syntax, that's not printing arrayHist correctly. I would like to review it and see where the errors are so I can correct them. Also, If you feel like you can offer me a more efficient methods, and means for this code, feel free to do so, I'll take it into consideration.

{
    public static void main(String[] args){
                int[] array=randomIntArray(30);//declares the dimensions of the array     "randomIntArray"
                System.out.println("Scores:");
                printArray(array);//calls the scoreHist method to print the "array variable
                System.out.println("\n"+"count: ");
                printArray(arrayHist(array));//calls score hist, to call arrayHist, to count "array"
                System.out.println();
    }

        public static int randomInt(int low, int high){//the prng
                double e;
                double x=Math.random();
                    e=low+x*(high-low);
                return (int)e;}

        public static int[] randomIntArray(int n){//generates an array of random numbers based on an upper and lower bound
                int[] a=new int[n];
                for(int i=0;i<a.length;i++)
                    {a[i]=randomInt(-5,15);}//"-5&15 are the lower and upper bounds of the random number array
                return a;}

        public static int inRange(int[] a, int low, int high){//checks if the numbers of the random number array are in certain constraints.
                int count=0;
                for(int i=0;i<a.length;i++)
                    {if(a[i]>=low&&a[i]<=high)count++;}
                return count;}

        public static int[] arrayHist(int[] scores){//counts how many of each number occurs in "randomIntArray"
                int[] counts=new int[11];
                for(int i=0;i<10;i++)
                    {counts[i]=inRange(scores,i,i++);}
                return counts;}

        public static void printArray(int[] a){//prints any array, with only one traversal.
                for(int i=0;i<a.length;i++)
                    {System.out.print(" "+a[i]);}
            }
}

My goal is to not use any of the array methods Java provides (to challenge myself), so please don't offer them up.

you need to add some spaces in between operators and format the code properly

all I did here was

added spaces between operators
moved the curly braces to their appropriate locations for Egyptian Style Bracing
set indentation to standard, you had some lines that were indented and they shouldn't have been

public static void main(String[] args) {
    int[] array = randomIntArray(30); //declares the dimensions of the array "randomIntArray"
    System.out.println("Scores:");
    printArray(array); //calls the scoreHist method to print the "array variable
    System.out.println("\n" + "count: ");
    printArray(arrayHist(array)); //calls score hist, to call arrayHist, to count "array"
    System.out.println();
}
public static int randomInt(int low, int high) { //the prng
    double e;
    double x=Math.random();
    e=low+x*(high-low);
    return (int)e;
}
public static int[] randomIntArray(int n) { //generates an array of random numbers based on an upper and lower bound
    int[] a = new int[n];
    for(int i = 0; i < a.length; i++) {
        a[i] = randomInt(-5, 15);
    } //"-5&15 are the lower and upper bounds of the random number array
    return a;
}
public static int inRange(int[] a, int low, int high) { //checks if the numbers of the random number array are in certain constraints.
    int count = 0;
    for(int i = 0; i < a.length; i++) {
        if(a[i] >= low && a[i] <= high) count++;
    }
    return count;
}
public static int[] arrayHist(int[] scores) { //counts how many of each number occurs in "randomIntArray"
    int[] counts=new int[11];
    for(int i=0;i<10;i++) {
        counts[i] = inRange(scores, i, i++);
    }
    return counts;
}
public static void printArray(int[] a) { //prints any array, with only one traversal.
    for(int i = 0; i < a.length; i++) {
        System.out.print(" " + a[i]);
    }
}

I noticed that you are using a For loop, and I think that you should be using more foreach loops.  I am going to use this code as an example

public static int inRange(int[] a, int low, int high) { //checks if the numbers of the random number array are in certain constraints.
    int count = 0;
    for(int i = 0; i < a.length; i++) {
        if(a[i] >= low && a[i] <= high) count++;
    }
    return count;
}

so what you get is now this

public static int inRange(int[] a, int low, int high) {
    int count = 0;
    for(int i : a) {
        if(i >= low && i <= high) count++;
    }
    return count;
}

which is slightly cleaner than the OP.

I also removed the comments on this method, mainly because they were unnecessary. you have a lot of comments in this code some of which are not needed at all, probably all of them.  most of your variables are named appropriately so you don't have a doubt what they stand for.  nothing is too complex here that you would need to explain what is going on.

here are the comments I would get rid of

//declares the dimensions of the array "randomIntArray"
//declares the dimensions of the array "randomIntArray"
//the prng

Here is the only comment that I would keep

//counts how many of each number occurs in "randomIntArray"

on the ArrayHist Method. because the Method name doesn't really tell me that right away.

All Methods should be named in PascalCase, variables in camelCase.


Reduce length of condition Is there a way to reduce this lengthy if condition:

if(
    statusCode.equals( NO_ERRORS_NO_WARNINGS ) ||
    statusCode.equals( HAS_ERRORS_NO_WARNINGS )  ||
    statusCode.equals( HAS_ERRORS_HAS_WARNINGS ) ||
    statusCode.equals( NO_ERRORS_HAS_WARNINGS )
){
    //do something
}

The purpose of it is to see if there is a status code set within a log file. So the String statusCode could be a null value, one of the 4 status codes above or an unpredicatable String.

First of all, if statusCode can be null, the code above could possibly throw a null pointer exception.

To directly answer your question though, you could use a Set, such as a HashSet.

HashSet<String> errorOrWarning = new HashSet<String>();
errorOrWarning.add(HAS_ERRORS_HAS_WARNINGS);
errorOrWarning.add(HAS_ERRORS_NO_WARNINGS);
errorOrWarning.add(HAS_ERRORS_HAS_WARNINGS);
errorOrWarning.add(NO_ERRORS_HAS_WARNINGS);

Which you can then use in a single call.

if (errorOrWarning.contains(statusCode) {
    // do something
}

This has the advantage that statusCode can be null and it would produce no exception.


Showing items and categories after clicking on a subcategory How can I speed up my app?  It's very slow if I click on a button and open a new page. All data in time is local. I'm not sure if the problem is in my code or xamarinu.

foreach (var category in avm.Categories)
    {
        ToolbarItem ItemCategory = new ToolbarItem();
        ItemCategory.Name = category.Name;
        ToolbarItems.Add (ItemCategory);
        ItemCategory.Activated += (object sender, EventArgs e) =>
        {
            SubCategoryStack.Children.Clear();
            this.IsPresented = true;
            foreach (var Subcategory in category.SubCategories)
            {
                Button btn_subcategory = new Button();
                btn_subcategory.Text = Subcategory.Name;
                SubCategoryStack.Children.Add(btn_subcategory);

                btn_subcategory.Clicked += (object senderr, EventArgs d) =>
                {
                    this.IsPresented= false;
                    stackIbro2.Children.Clear();
                    stackIbro.Children.Clear();
                    Array array = Array.CreateInstance(typeof(Article), Subcategory.Articles.Count);
                    for(int i = 0; i< Subcategory.Articles.Count; i++)
                    {
                        StackLayout articlesH = new StackLayout()
                        {Orientation = StackOrientation.Horizontal};
                        Button btn_article = new Button();
                        Label btn_Quantity = new Label();
                        array.SetValue(Subcategory.Articles[i], i);
                        btn_article.Text = Subcategory.Articles[i].Name;
                        btn_Quantity.Text = Subcategory.Articles[i].Quantity.ToString();

                        btn_article.HorizontalOptions = LayoutOptions.StartAndExpand;
                        btn_Quantity.HorizontalOptions = LayoutOptions.End;
                        articlesH.Children.Add(btn_article);
                        articlesH.Children.Add(btn_Quantity);
                        stackIbro2.Children.Add(articlesH);
                    }

                    //btn_article.Text = new System.Collections.ObjectModel.ObservableCollection<Article> ((Article[])array).ToString();

                };

            }
        };
    }

Naming

Assuming you or Mr.Maintainer will have to fix a bug in this code in 3 months, will you still know what any of these variables

avm
stackIbro2
stackIbro
articlesH

should do or represent ?

You should give your objects/variables meaningful names so that you or Mr.Maintainer will see at the first glance what to expect.

Indentation

                    StackLayout articlesH = new StackLayout()
                    {Orientation = StackOrientation.Horizontal};

here it can be missed at first glance that these 2 lines belong together. You should indent them like

                    StackLayout articlesH = new StackLayout()
                        {Orientation = StackOrientation.Horizontal};

btn_subcategory.Clicked eventhandler

Grouping together the related code, will increase readability.
the Array array is not really needed at all, you can remove it
commented code is dead code, hence it should be removed.
changing the for loop to a for each loop will add readability
EventArgs are name e and it is sender instead of senderr
don't use hungarian notation (at least like you have done). If the type changes but you forget to change the name also you get weird code like: Label btn_Quantity

btn_subcategory.Clicked += (object sender, EventArgs e) =>
{
    this.IsPresented= false;
    stackIbro2.Children.Clear();
    stackIbro.Children.Clear();

    foreach (Article article in Subcategory.Articles)
    {
        StackLayout articlesH = new StackLayout()
            {Orientation = StackOrientation.Horizontal};

        Button button = new Button()
            {
                Text = article.Name,
                HorizontalOptions = LayoutOptions.StartAndExpand
            }

        Label label = new Label()
            {
                Text = article.Quantity.ToString(),
                HorizontalOptions = LayoutOptions.End
            }

        articlesH.Children.Add(button;
        articlesH.Children.Add(label);
        stackIbro2.Children.Add(articlesH);
    }
};


login_required decorator in Flask I have 2 Flask apps (different projects) that work together . One implements some API which uses tokens for auth. The second one consumes the API and makes a web interface for it. Now I have a login function that sends the username and password to the API, and if correct, gets the auth token in return. Once I have the token, I save it to the session of the user and the user should now be considered as logged in/ autheticated.

Here is my login function -

def login(self):
        response = make_request(BASE_URL + 'login/', clean_data(self.data))
        if response.status_code == 200:
            session['auth_token'] = response.json().get('auth_token')
            return True
        return False

Also I am using Redis to store sessions if that matters. I have made the following login_required decorator -

def login_required(f):
    @wraps(f)
    def decorated_function(*args, **kwargs):
        if session.get('auth_token') is None:
            return redirect(url_for('login', next=request.url))
        return f(*args, **kwargs)

    return decorated_function

While it does work, I'm not sure if this is the best way to make that decorator. Any suggestions would be helpful.

I would suggest using:

if 'auth_token' not in session:
    ...

Since your login function suggests that auth_token is not a key in session, unless the user has logged in.

Other that that, I can see no other improvements.


Flow control with try catch to reduce redundancy I've seen a similar question to this but mine's a little more specific. I have the below

File f = new File("./data.txt");
if(f.isFile())
{
    try
    {
        FileChannel fc = new RandomAccessFile(f,"r").getChannel();
        //Get information and return it
    }
    catch(FileNotFoundException e)
    {
        System.out.println("FileNoFoundException: "+e.getMessage());
    }
}
else
{
    //If the file wasn't found, return default values
}

I was wondering, should I just put the default returning of the information in the catch block? I assumed that'd be a no because you're not really supposed to use it to control flow. But having the both the if and the try catch just looks too redundant to me.

In cases like this, I like taking a default-unless-better approach... Consider this re-working of your code to do the same thing in a different order.

The basic premise is:

set up a default value (or null if it is a complicated thing to do).
do the hard work which may be missing dependencies or may fail
if the hard work completes successfully, return that result.

The reason this works well is because you can exit from the hard work at any point, and have the default standing by to continue.....

... also, as an exercise, use the try-with-resources and new NIO2 features in Java7

// set up the default value...
SomeValue result = null;
Path path = Paths.get("./data.txt");
if (Files.isRegularFile(path)) {
    try (SeekableByteChannel channel =
            Files.newByteChannel(path, StandardOpenOptions.READ)) {

        // do the work required for your file....
        ...

        // after everything is successful... set the result:
        result = new SomeValue(....real arguments ....);
    } catch (IOException ioe) {
        System.out.println("FileNoFoundException: " + e.getMessage());
    }
}
result == null ? new SomeValue(defaults...) : result;

Additionally, there is no reason, if you are in a method that builds these things, that you can't return immediately with the right answer.

Often if you are doing things like the above, it indicates that what you are doing should be in a sub-method, with an early-return when you have a successful setup, and a default return value when things fail.


TaggedLogger - Log wrapper I wrote this class in order to not having to write tag every time (because of code duplication and risk of typos).

public class TaggedLogger {

    public static final TaggedLogger GLOBAL = new TaggedLogger("Global");

    private final String tag;

    public static TaggedLogger forClass(Class<?> someClass) {
        return new TaggedLogger(someClass.getName());
    }

    public TaggedLogger(String tag) {
        this.tag = tag;
    }

    public void debug(String message) {
        Log.d(tag, message);
    }

    public void info(String message) {
        Log.i(tag, message);
    }

    public void exception(String message) {
        Log.e(tag, message);
    }

    public void exception(String message, Throwable exception) {
        Log.e(tag, message, exception);
    }

    // skipped

}

Using:

public class MainActivity extends Activity {
    private static final TaggedLogger LOGGER = TaggedLogger.forClass(MainActivity.class);

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);
        LOGGER.debug("Initialization: start");
        init(savedInstanceState);
    }

    // skipped

}

What do you think? Maybe this class is useless?

Help me choose a more appropriate name for this class (if TaggedLogger isn't good) ?

I do not consider your class useless, I have actually done a similar thing myself. Very similar actually. Perhaps even exactly the same, pretty much.

Overall, your code is very well-written. You use final and private where you can, which I like.

A few comments:

private static final TaggedLogger LOGGER = TaggedLogger.forClass(MainActivity.class);

Even though it is correct that constants are written in ALL_CAPS. One exception for this is loggers. If you write LOGGER.info and stuff often, that LOGGER will pick on your eyes after a while. I would recommend using the name logger (lowercase) instead.

public static final TaggedLogger GLOBAL = new TaggedLogger("Global");

I am not sure if you really need this one. Especially not inside that class.

It is good that you have not made your class a singleton at least, but I'm not sure if this public static 'global' is really necessary.

I can recommend using log4j for Android for logging. Log4j is a very nice and popular logging framework for Java, and it is possible to get it working on Android.


Simple Email Subject Filtering (python) This challenge took me forever.  I spent a lot of time messing with regular expressions, but couldn't get it to work.  I ended up coming up with this abomination.  If anybody wants to take the time I'm interested in simpler ways to accomplish this task. (I'm a brand new pythoner)

def is_stressful(subj):
    """
        recognise a stressful email subject
        we are looking for any of: all uppercase, or ending in 3 !!!, or
        containing 'help', 'asap', or 'urgent' despite have extraneous spellings
    """

    import string

    flagged_words = ['help', 'asap', 'urgent']

    if subj[-3:] == '!!!':  # check for ending in at least 3 !!!
        return True
    if subj.isupper():   # check for uppercase
        return True

    stripped = "".join(c for c in subj if c not in string.punctuation) # get rid of confusing characters
    wordlist = stripped.lower().split(' ') # make the list to check

    for word in wordlist:  # first easy check, everything is spelled correctly
        if word in flagged_words:
            return True
    for word in wordlist:  # start the annoying check, getting rid of extra letters in flagged words
        r = 0  # our word list counter
        getgood = []
        badletters = []
        while r < len(flagged_words):  # start the loop
            for i, l in enumerate(word):  # going through each character
                if l in flagged_words[r]:  # checking for good letters
                    if l not in getgood:
                        getgood.append(l)  # no repeats except for 'a' in asap (don't know a good way for this)
                    elif l == 'a' and getgood.count('a') < 2:
                        getgood.append(l)
                else:
                    badletters.append(False)  # make sure we don't spell flagged words accidentally
            r += 1  # go to the next word in list
            i = 0  # reset letter counter
            if ''.join(getgood) in flagged_words and all(badletters):  # our final check
                return True
            getgood = []  # reset the loop
            badletters = []  # reset the loop
    return False

My tests:

print(is_stressful("H!E!L!P! its urGent asAP"))
print(is_stressful("asaaap"))
print(is_stressful("Headlamp, wastepaper bin and supermagnificently"))
print(is_stressful("I neeed advice!!!!"))

Thanks for any tips!

Improvements with a new solution:

import string. If the function is a top-level commonly used function - better to move that import at the top of enclosing module.Though in my proposed solution string won't be used.
flagged_words. Instead of generating a list of flagged words on every function's call - make it a constant with immutable data structure defined in the outer scope:

FLAGGED_WORDS = ('help', 'asap', 'urgent')

empty subj. To avoid multiple redundant checks/conditions on empty subj argument, if such would be passed in, a better way is handling an empty string at start:

subj = subj.strip()
if not subj:
    raise ValueError('Empty email subject!')

if subj[-3:] == '!!!' and if subj.isupper() checks lead to the same result.That's a sign for applying Consolidate conditional expression technique - the conditions are to be combined with logical or operator
the last for ... while ... for traversal looks really messy and over-complicated.When trying to untangle that, your test cases allowed me to make an assumption that the crucial function (besides of trailing !!! chars and all upper-cased letters) should catch:

exact word match with any of the flagged words, like urGent asAP (test case #1)
exact word match with repetitive allowed chars like asaaap (test case #2)

and should not allow strings that contain only words which combine both allowed and unallowed chars like Headlamp or wastepaper (though they contain he..l..p, .as....ap..) (test case #3)

Instead o going into a mess of splitting/loops I'd suggest a complex regex solution that will cover both exact word matching and exact matching with repetitive allowed chars cases.The underlying predefined pattern encompasses the idea of quantifying each char in each flagged word like h+e+l+p+ with respect for word boundaries \b:

import re

FLAGGED_WORDS = ('help', 'asap', 'urgent')

def quantify_chars_re(words):
    """Adds `+` quantifier to each char for further use in regex patterns"""
    return [''.join(c + '+' for c in w) for w in words]

RE_PAT = re.compile(fr"\b({'|'.join(quantify_chars_re(FLAGGED_WORDS))})\b", re.I)

def is_stressful(subj):
    """
        Recognize a stressful email subject
        we are looking for any of: all uppercase, or ending in 3 !!!, or
        containing 'help', 'asap', or 'urgent' despite have extraneous spellings
    """

    subj = subj.strip()
    if not subj:
        raise ValueError('Empty email subject!')

    if subj.isupper() or subj[-3:] == '!!!' or RE_PAT.search(subj):
        return True

    return False

if __name__ == "__main__":
    print(is_stressful("H!E!L!P! its urGent asAP"))
    print(is_stressful("asaaap"))
    print(is_stressful("Headlamp, wastepaper bin and super-magnificently"))
    print(is_stressful("I neeed advice!!!!"))

The output:

True
True
False
True


Sending a C++ array to Python/NumPy and back I am going to send a C++ array to a Python function as NumPy array and get back another NumPy array. After consulting with NumPy documentation and some other threads and tweaking the code, the code is finally working but I would like to know if this code is written optimally considering the:

Unnecessary copying of the array between C++ and Numpy (Python).
Correct dereferencing of the variables.
Easy straight-forward approach.

C++ code:

// python_embed.cpp : Defines the entry point for the console application.
//

#include "stdafx.h"

#define NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION
#include "Python.h"
#include "numpy/arrayobject.h"
#include<iostream>

using namespace std;

int _tmain(int argc, _TCHAR* argv[])
{
    Py_SetProgramName(argv[0]);
    Py_Initialize();
    import_array()

    // Build the 2D array
    PyObject *pArgs, *pReturn, *pModule, *pFunc;
    PyArrayObject *np_ret, *np_arg;
    const int SIZE{ 10 };
    npy_intp dims[2]{SIZE, SIZE};
    const int ND{ 2 };
    long double(*c_arr)[SIZE]{ new long double[SIZE][SIZE] };
    long double* c_out;
    for (int i{}; i < SIZE; i++)
        for (int j{}; j < SIZE; j++)
            c_arr[i][j] = i * SIZE + j;

    np_arg = reinterpret_cast<PyArrayObject*>(PyArray_SimpleNewFromData(ND, dims, NPY_LONGDOUBLE,
        reinterpret_cast<void*>(c_arr)));

    // Calling array_tutorial from mymodule
    PyObject *pName = PyUnicode_FromString("mymodule");
    pModule = PyImport_Import(pName);
    Py_DECREF(pName);
    if (!pModule){
        cout << "mymodule can not be imported" << endl;
        Py_DECREF(np_arg);
        delete[] c_arr;
        return 1;
    }
    pFunc = PyObject_GetAttrString(pModule, "array_tutorial");
    if (!pFunc || !PyCallable_Check(pFunc)){
        Py_DECREF(pModule);
        Py_XDECREF(pFunc);
        Py_DECREF(np_arg);
        delete[] c_arr;
        cout << "array_tutorial is null or not callable" << endl;
        return 1;
    }
    pArgs = PyTuple_New(1);
    PyTuple_SetItem(pArgs, 0, reinterpret_cast<PyObject*>(np_arg));
    pReturn = PyObject_CallObject(pFunc, pArgs);
    np_ret = reinterpret_cast<PyArrayObject*>(pReturn);
    if (PyArray_NDIM(np_ret) != ND - 1){ // row[0] is returned
        cout << "Function returned with wrong dimension" << endl;
        Py_DECREF(pFunc);
        Py_DECREF(pModule);
        Py_DECREF(np_arg);
        Py_DECREF(np_ret);
        delete[] c_arr;
        return 1;
    }
    int len{ PyArray_SHAPE(np_ret)[0] };
    c_out = reinterpret_cast<long double*>(PyArray_DATA(np_ret));
    cout << "Printing output array" << endl;
    for (int i{}; i < len; i++)
        cout << c_out[i] << ' ';
    cout << endl;

    // Finalizing
    Py_DECREF(pFunc);
    Py_DECREF(pModule);
    Py_DECREF(np_arg);
    Py_DECREF(np_ret);
    delete[] c_arr;
    Py_Finalize();
    return 0;
}

Python Code:

def array_tutorial(a):
    print("a.shape={}, a.dtype={}".format(a.shape, a.dtype))
    print(a)
    a *= 2
    return a[-1]

1. Review

It's not recommended to use using namespace std; — the problem is that this imports all of the identifiers from std, and some of these may shadow names from other modules that you need to use. See this question on Stack Overflow.
The code doesn't check for success/failure of many of the functions it calls. These can all fail:

new long double[SIZE][SIZE]
PyArray_SimpleNewFromData
PyUnicode_FromString
PyTuple_New
PyTuple_SetItem
PyObject_CallObject

(and maybe others that I didn't spot).
There's no Py_DECREF for pArgs.
It would be possible to avoid constructing the argument tuple, by using PyObject_CallFunctionObjArgs instead of PyObject_CallObject.
Error messages should be written to standard error (cerr), not standard output.
It's clearer to use the macros EXIT_SUCCESS and EXIT_FAILURE from <cstdlib> instead of 0 and 1.
The code calls PyArray_NDIM and PyArray_SHAPE on np_ret without checking to see if this object is in fact an array. Call PyArray_Check first.
Using value initialization for int variables seems perverse to me. With int i{} you have to remember that the default constructor gives the variable the value 0. With int i = 0 there's no need to remember (and the code is no less efficient: the compiled code doesn't actually create a temporary 0 object and then call the int assignment constructor).
This code doesn't provide as much information about errors as it could. In particular, errors inside Python cause Python to create an exception object containing information about the error (see "Exception Handling"). It would be a good idea to print this object if it exists, by checking PyErr_Occurred and then calling PyErr_Print.
Each block of error handling code has to undo the effect of all the previous successful blocks of code. This makes the length of the function quadratic in the number of error cases! This is risky, because every time you change something, you have to adjust all the error cases accordingly, and it is very easy to forget (as in §1.3 above). Also, the pain of writing out all the error-handling code makes it tempting to skip error handling for functions that you believe are likely to succeed (as in §1.2 above).

See the revised code below for one way to arrange that each "undo" operation appears just once.

2. Revised code

This is untested, so probably has some errors.

int _tmain(int argc, _TCHAR* argv[])
{
    int result = EXIT_FAILURE;

    Py_SetProgramName(argv[0]);
    Py_Initialize();
    import_array();

    // Build the 2D array in C++
    const int SIZE = 10;
    npy_intp dims[2]{SIZE, SIZE};
    const int ND = 2;
    long double(*c_arr)[SIZE]{ new long double[SIZE][SIZE] };
    if (!c_arr) {
        std::cerr << "Out of memory." << std::endl;
        goto fail_c_array;
    }
    for (int i = 0; i < SIZE; i++)
        for (int j = 0; j < SIZE; j++)
            c_arr[i][j] = i * SIZE + j;

    // Convert it to a NumPy array.
    PyObject *pArray = PyArray_SimpleNewFromData(
        ND, dims, NPY_LONGDOUBLE, reinterpret_cast<void*>(c_arr));
    if (!pArray)
        goto fail_np_array;
    PyArrayObject *np_arr = reinterpret_cast<PyArrayObject*>(pArray);

    // import mymodule.array_tutorial
    const char *module_name = "mymodule";
    PyObject *pName = PyUnicode_FromString(module_name);
    if (!pName)
        goto fail_name;
    PyObject *pModule = PyImport_Import(pName);
    Py_DECREF(pName);
    if (!pModule)
        goto fail_import;
    const char *func_name = "array_tutorial";
    PyObject *pFunc = PyObject_GetAttrString(pModule, func_name);
    if (!pFunc)
        goto fail_getattr;
    if (!PyCallable_Check(pFunc)){
        std::cerr << module_name << "." << func_name
                  << " is not callable." << std::endl;
        goto fail_callable;
    }

    // np_ret = mymodule.array_tutorial(np_arr)
    PyObject *pReturn = PyObject_CallFunctionObjArgs(pFunc, pArray, NULL);
    if (!pReturn)
        goto fail_call;
    if (!PyArray_Check(pReturn)) {
        std::cerr << module_name << "." << func_name
                  << " did not return an array." << std::endl;
        goto fail_array_check;
    }
    PyArrayObject *np_ret = reinterpret_cast<PyArrayObject*>(pReturn);
    if (PyArray_NDIM(np_ret) != ND - 1) {
        std::cerr << module_name << "." << func_name
                  << " returned array with wrong dimension." << std::endl;
        goto fail_ndim;
    }

    // Convert back to C++ array and print.
    int len = PyArray_SHAPE(np_ret)[0];
    c_out = reinterpret_cast<long double*>(PyArray_DATA(np_ret));
    std::cout << "Printing output array" << std::endl;
    for (int i = 0; i < len; i++)
        std::cout << c_out[i] << ' ';
    std::cout << std::endl;
    result = EXIT_SUCCESS;

fail_ndim:
fail_array_check:
    Py_DECREF(pReturn);
fail_call:
fail_callable:
    Py_DECREF(pFunc);
fail_getattr:
    Py_DECREF(pModule);
fail_import:
fail_name:
    Py_DECREF(pArray);
fail_np_array:
    delete[] c_arr;
fail_c_array:
    if (PyErr_Check())
        PyErr_Print();
    PyFinalize();
    return result;
}


PHP PDO Factory Classes So, my question is more of a 'best practices' question rather than a question with a particular aim. This is my current understanding of PHP factories and how to incorporate them into a project using PDOs and dependency injection. I'm pretty new at this and I still don't understand a lot about this subject. As such, I really don't know if I'm doing this quite right. Could someone point me in the right direction?

<?php
class PDOSettings_Factory {
    private function __construct() {}
    public static function build(Array $args = NULL) {
        $settings = array();
        // Set default values.
        $settings["dbType"] = 'mysql';
        $settings["dbName"] = 'test';
        $settings["host"] = 'localhost';
        $settings["user"] = 'guest';
        $settings["pass"] = '1234';
        // If keys are the same, they will be overwritten.
        if(isset($args)) {
            foreach($args as $key => $val) {
                $settings[$key] = $val;
            }
        }
        // Return setings.
        return $settings;
    }
}

$settings = PDOSettings_Factory::build();

class PDOConnection_Factory {
    private function __construct() {}
    public static function build($args) {
        // Set up PDO connection
        if(!isset($args)) {
            return NULL;
        } else {
            $statement =
                $args['dbType'].':'.
                'host='.$args['host'].';'.
                'dbname='.$args['dbName'];
            try {
                $dbh = new PDO(
                    $statement,
                    $args['user'],
                    $args['pass']);
                $dbh->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
                // Return DBH
                return $dbh;
            } catch (PDOException $e) {
                echo 'Error: ',  $e->getMessage(), "\n";
            }
        }
        return NULL;
    }
}

And how to use the factories:

class demo {
    private $dbh = NULL;
    public function set($args) {
        if (!isset($args)) {return FALSE;}
        else {
            $this->dbh = $args;
            return TRUE;
        }
    }
    public function __construct($args = NULL) {
        $this->set($args);
    }

    public function doSomething() {
        // Set SQL command here.
        $sql = 'SELECT * FROM test';

        $results = $this->dbh->prepare($sql);
        // Perform param binds here.
        $results->execute();
        return $results;
    }
}

$db = PDOConnection_Factory::build($settings);

$test = new demo($db);
foreach ($test->doSomething() as $row) {print_r($row[0].PHP_EOL);}
?>

Formatting strings

You should try to avoid those kind of things:

$statement = $args['dbType'].':'.'host='.$args['host'].';'.'dbname='.$args['dbName'];

It's easy to miss something, and it's also this kind of constructions that leads to difficult to read code and (in other contexts) SQL injections. I would argue that using sprintf is a better idea here:

$format = '%s:host=%s:dbname=%s';
echo sprintf($format, $args['dbType'], $args['host'], $args['dbName']);

I don't know if it is considered idiomatic PHP or not, but it seems more readable and less error-prone to me.

PDO and exceptions

mnhg already gave you a way to get rid of the first return NULL in PDOConnection_Factory::build(), but it's not enough, you should also remove the second one. I assume it comes from the possible exceptions in PDO::__construct and PDO::setAttribute. The issue is that you're catching the exception but doing nothing with them.

You should instead embrace exceptions. :) A good first step is the setAttribute call. Since there's nothing you can do to handle them in the factory: you should instead let the application code decide what to do if something fails: for example, display a warning message to users and send an email to the administrator.


Correct use of Serializable with lambda functions I'm practicing saving and reading serializable objects when a friend told me serializable objects have no use in real life, that I'd better focus on JSON or XML to serialize my objects when I came across this situation:
I want to preload some FileFilters to use them in a JFileChooser dialog and I want these Filefilters to have more complicated behaviour than just file extension.
The code works fine but I've read there could be some security issues saving executable code and reading it later. Is it true? Can they be avoided?
I had to implement my own interface of Predicate due to the fact that Predicate is not Serializable.
import java.io.File;
import java.io.Serializable;

public interface MyPredicate extends Serializable {
  boolean isValid(File file);
}

This is the class that defines how the filters are going to be set (It's called List despite the fact that uses internally a Map just in case implementation changes).
import java.io.Serializable;
import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;

public class FileFilterList implements Serializable {
  private Map<String, MyPredicate> filters;

  public FileFilterList() {
    this.filters = new HashMap<>();
  }

  public FileFilterList addFilter(String string, MyPredicate predicate) {
    this.filters.put(string, predicate);
    return this;
  }

  public Iterator<Map.Entry<String, MyPredicate>> getIterator() {
    return this.filters.entrySet().iterator();
  }
}

In this little program I save a list of filters in a file
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.ObjectOutputStream;

public class SaveFileFilter {
  public static void main(String[] args) throws IOException {
    FileFilterList filterList = new FileFilterList();
    filterList
            .addFilter("txt, Text files", f -> f.isDirectory() || f.getName().endsWith(".txt"))
            .addFilter("Big txt, Big txt files (>1Kb)", f -> f.isDirectory() || (f.getName().endsWith(".txt") || f.length() > 1024))
            .addFilter("Small txt, Small txt file (<=1Kb)", f -> f.isDirectory() || (f.getName().endsWith(".txt") || f.length() <= 1024));
    try (ObjectOutputStream oos = new ObjectOutputStream(new FileOutputStream("filters.obj"))) {
      oos.writeObject(filterList);
    } catch (IOException ioe) {
      System.err.println("Sorry, there wa a problem saving filters");
      ioe.printStackTrace();
      throw ioe;
    }
  }
}

And this would be the main program that get the preset file filter list:
import javax.swing.*;
import javax.swing.filechooser.FileFilter;
import java.io.*;

public class FileFilterUse {
  public static void main(String[] args) throws IOException, ClassNotFoundException {
    JFileChooser fileChooser = new JFileChooser();
    try (ObjectInputStream ois = new ObjectInputStream(new FileInputStream("filters.obj"))) {
      FileFilterList filterList = (FileFilterList) ois.readObject();
      filterList.getIterator().forEachRemaining(
              (entry) -> fileChooser.addChoosableFileFilter(new FileFilter() {
                @Override
                public boolean accept(File f) {
                  return entry.getValue().isValid(f);
                }

                @Override
                public String getDescription() {
                  return entry.getKey();
                }
              }));
      fileChooser.showOpenDialog(null);
    } catch (IOException ioe) {
      System.err.println("There was a problem reading filters file");
      ioe.printStackTrace();
      throw ioe;
    } catch (ClassNotFoundException cnf) {
      System.err.println("File format is not correct");
      cnf.printStackTrace();
      throw cnf;
    }
  }
}

Every java file has a package line.
I guess it is not possible to achive this serializing objects to JSON or XML, is it?
By the way, any comment about the code would be very appreciated.

I'm  going to answer my own question. Never do this!. I have just tried saving a new Filefilter object with this filter:
.addFilter("Delete files, Delete all files",f->f.delete());

And when selected, it deletes all the files in the current directory.


Create JIRA ticket using REST API Objective: The objective of the program is to create a JIRA ticket using a REST API.  It's working perfectly fine without any issue. However I am not sure if it's up to good Scala standards.

I would be delighted if someone could let me know what I can do to improve it.

import org.apache.http.HttpResponse
import org.apache.http.auth.UsernamePasswordCredentials
import org.apache.http.client.methods.HttpPost
import org.apache.http.entity.StringEntity
import org.apache.http.impl.auth.BasicScheme
import org.apache.http.impl.client.HttpClients
import org.apache.http.util.EntityUtils

object JIRAUtil{

  private def getJiraID(response: HttpResponse): String ={
    val entity = response.getEntity
    val content = EntityUtils.toString(entity)
    val id = "DPPR-\\d{4}".r
    val jiraID = id.findFirstIn(content)
    assert(jiraID.isDefined, s"Failed to create JIRA: ${content}")
    jiraID.get
  }

  def createJIRA(project: String, summary: String, description: String, assigneeName: String,
                 assigneeEmail: String): String = {
    val data = s"""{
      "fields": {
         "project":{
            "key": "${project}"
         },
         "summary": "[Automation] - ${summary}",
         "customfield_10118": "DPPR-1109",
         "assignee": {
            "name": "${assigneeName}",
            "emailAddress": "${assigneeEmail}"
         },
         "description": "${description}",
         "issuetype": {
            "name": "Bug"
         }
     }
    }"""

    val url = "https://acosta-it.atlassian.net/rest/api/latest/issue/"
    val post = new HttpPost(url)
    val httpClient = HttpClients.createDefault()

    val credential = new UsernamePasswordCredentials("gaurang.shah@abc.com", "CoddadEhptnDfU18F")
    post.addHeader("Content-type", "application/json")
    post.addHeader(new BasicScheme().authenticate(credential, post, null))
    post.setEntity(new StringEntity(data))
    val response = httpClient.execute(post)
    val jiraId = getJiraID(response)
    jiraId
  }

}

The suggestions I'd make are all around making the code more robust and more explicit in error handling.

Representing errors

Your methods return Strings. However, I'd say that's not really a good representation of what the methods do. Here's what I mean...

getJiraID can fail. You've checked for this in an assert, which is great. That will give an informative message, but will also stop the program. (It's totally possible that's OK for your case, but in general, it's not). What you could do instead is have the methods return perhaps an Either[Error, String] where Error could be a case class that captures the relevant details of what went wrong.

Why is that important? If I'm using your code, the signature says I'll get a String back no matter what I pass in. However, what really happens is that the programme could stop. That would be very surprising to me as a user.

If, instead, I see the signature is Either[Error, String] I know it could fail, and I have to handle that appropriately.

To be clear: I'm making a general point. Your getJiraID is private, and you could say it's an internal helper, and will never fail in reality, but hopefully, you see the point I'm making.

That carries through into createJIRA which also returns a String (no matter what), but clearly this can fail. It can fail in more ways: presumably, the http client you're using could fail due to network error or a server error.  This is essentially the same point I made above.

JSON

The value data represents JSON. What happens if project contains a quotation mark? You'll have invalid JSON. I'd suggest using a JSON library to construct JSON safely, which will handle the cases of properly encoding String. Circe is a good one, but there are plenty of others.

Hardcoded credentials

You have included a username and password in the body of the code. When that password expires (or changes), you'll have to hunt that down. I'd suggest making those parameters. For example:

case class JiraClient(user: String, pass: String) {
  def create(...) = ...
}

... and then creating a JiraClient at the edges of your code (e.g., in a main) and perhaps reading those credentials from the environment (command-line arguments, configuration file, environment variables). You'd then have something anyone could use, and is easy to change without a recompile.

That same suggestion might apply for the url, too. I don't know how often that might change.

Trivial

Some of the formatting looks a little odd to my eye. E.g., String ={ vs. String = {. Take a look at scalafmt as a way to automatically format your code for consistency.


'StrictInt' Python object class that prohibits casting numbers with non-integer components to `int` Inspired by and created because of python3 utility: convert safely to int and driven partly by the 'refactor' in my answer.

The aforementioned question and my answer drove me to go and do a thing while bored at work - I created a new object class called StrictInt that behaves much like the function created by the original poster of that question/thread, but as an object class instead.

This object class is a subclass of int (which is  the superclass.  It's designed to work just like an int, but when you cast a str, int, or float to the StrictInt object, it will only convert the value if it is indeed an integer.

Because I'm crazy, you'll also need the typing module, but most Python 3 installs have that, if not, grab it from pip3.

This is the code used for the StrictInt class, and any required imports, in the strictint.py file:

from typing import Union, Any

class StrictInt(int):
    def __new__(cls, value, *args, **kwargs):
        # type: (Union[int, float, str], Any, Any) -> int

        if not isinstance(value, (int, StrictInt, float, str)):
            t = str(type(value)).replace("<class '", "").replace("'>", "")
            raise TypeError("Cannot convert type '{type}' to strict integer".format(type=t))

        try:
            f = float(value)
        except ValueError:
            f = None

        if not f:
            raise ValueError("Cannot convert a non-number to a strict integer.")

        if not f.is_integer():
            raise ValueError("Cannot convert value due to non-integer parts.")

        return super(StrictInt, cls).__new__(cls, int(f))

There is a set of unit tests I've been using as well (tests.py):

from strictint import StrictInt
import unittest

class TestStrictInt(unittest.TestCase):

    def test_float_conversion(self):
        # Non-integer parts present in a float, should raise ValueError
        self.assertRaises(ValueError, StrictInt, 3.14159)
        # Float that is equal to an int should be equal.
        self.assertEqual(3.0, StrictInt(3.0))

    def test_ints(self):
        # int(3) should equal StrictInt(3).
        self.assertEqual(3, StrictInt(3))

    def test_nonnumeric_string(self):
        # Not a number at all.
        self.assertRaises(ValueError, StrictInt, "I Am A Teapot")
        # Number with an invalid character in it, so Not a Number.
        self.assertRaises(ValueError, StrictInt, " 3.14159")
        # Has numeric content, but not a valid number due to dots.
        self.assertRaises(ValueError, StrictInt, "3.14.156")

    def test_numeric_string(self):
        # int('3') should equal StrictInt('3')
        self.assertEqual(int('3'), StrictInt('3'))
        # int(float('3.0')) is equal to int(3.0), and should equal StrictInt('3.0')
        self.assertEqual(int(float('3.0')), StrictInt('3.0'))
        # String with a number that has a decimal part should raise ValueError
        self.assertRaises(ValueError, StrictInt, '3.14159')

if __name__ == '__main__':
    unittest.main(warnings='ignore')

Any and all improvement suggestions are welcome.  This works pretty well and fairly quickly from what I've tested, but I value everyone's opinions.

Converting everything via a float means that you get the wrong result whenever the input cannot be represented exactly as a double-precision floating-point number. For example, this is surely not acceptable:

>>> StrictInt(10**23)
99999999999999991611392

There's an OverflowError when the input is too large to be represented as a float:

>>> StrictInt(10**400)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "cr195375.py", line 13, in __new__
    f = float(value)
OverflowError: int too large to convert to float

The strategy of enumerating the allowable types (int, StrictInt, float, str) means that many plausible use cases are disallowed, for example with fractions.Fraction:

>>> from fractions import Fraction
>>> StrictInt(Fraction(10, 1))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "cr195375.py", line 10, in __new__
    raise TypeError("Cannot convert type '{type}' to strict integer".format(type=t))
TypeError: Cannot convert type 'fractions.Fraction' to strict integer

and similarly with decimal.Decimal.
The Pythonic approach is to use duck typing — that is, instead of testing whether input belongs to a fixed collection of types, you call the appropriate methods on the input. Here we want to determine whether the input has a non-zero fractional part, and otherwise to use the integer part. Hence we need Python's built-in divmod function:

quotient, remainder = divmod(value, 1)
if remainder:
    raise ValueError("could not convert value due to non-zero "
                     f"fractional part: {val!r}")

There is no point taking *args and **kwargs if you are not going to use them.

Putting this together:

class StrictInt(int):
    "Subclass of int that refuses to coerce non-integer values."
    def __new__(cls, value):
        if isinstance(value, str):
            for converter in (int, float, complex):
                try:
                    value = converter(value)
                    break
                except ValueError:
                    pass
            else:
                raise ValueError(f"invalid literal for {cls.__name__}(): "
                                 f"{value!r}")
        if value.imag:
            raise ValueError("could not convert value due to non-zero "
                             f"imaginary part: {value!r}")
        quotient, remainder = divmod(value.real, 1)
        if remainder:
            raise ValueError("could not convert value due to non-zero "
                             f"fractional part: {value!r}")
        return super(StrictInt, cls).__new__(cls, int(quotient))

This handles a wider ranges of input types:

>>> StrictInt(Fraction(10, 2))
5
>>> StrictInt(Decimal('11.00000'))
11
>>> StrictInt(5+0j)
5

and it copes with integers that are too large to be represented as floats:

>>> StrictInt(10**400) == 10**400
True