A utility class for running up to two Tasks serially, where the last task to be queued "wins" Edit: Use cases for this class - I needed this class in the case where I had processes I needed to execute (in the form of tasks - they were all asynchronous) that I needed to issue to a UI control when certain events occurred. However, these events are something I dont control and can sometimes occur in close succession. This class is used to make sure only the last event to have issued the task will "win" and run the task only once.

This class lets you run multiple tasks serially. If a task is already running (Task1), the next task (Task2) is "queued". However, if, before Task1 completes, Task3 is executed, it will replace Task2 and will be executed next.

The class is NOT thread safe.

/// <summary>
/// Lets you run multiple tasks serially. If a task is already running (Task1), the next task (Task2) is "queued". However, if,
/// before Task1 completes, Task3 is executed, it will replace Task2 and will be executed next.
/// THe class is NOT thread safe.
/// </summary>
public class LastTaskWins
{
    private object m_pendingToken;
    private Task m_currentTask;

    public async Task<bool> TryRunAsync(Func<Task> taskFunc)
    {
        bool result = true;

        // If there is no task running, just run the one queued.
        if (m_currentTask == null)
        {
            await ExecuteTaskNowAsync(taskFunc);
        }
        else
        {
            // Save a token so we know if we can run.
            object token = m_pendingToken = new object();

            // Make sure the current task completes.
            await m_currentTask;

            // Check to see if a new task has been "queued".
            result = token == m_pendingToken;
            if (result)
            {
                // If we can run (we own the last-queued task token), run the new task.
                await ExecuteTaskNowAsync(taskFunc);
            }

        }

        return result;
    }

    /// <summary>
    /// Helper method. Will run the relevant task, maintaining class state.
    /// </summary>
    private async Task ExecuteTaskNowAsync(Func<Task> taskFunc)
    {
        m_currentTask = taskFunc();
        await m_currentTask;
        m_currentTask = null;
    }
}

I suggest to make m_currentTask non-nullable by replacing usages of null with a completed task. It will simplify the code as you can always await on this field, and assume it is the "current" task running.

You would need to introduce locking in order to synchronize access to the continuation task, and ensure that the taskFunc is only called when it was a winning bid. Also, you would need a double-checked locking to ensure that your task is the winner:

public class LastTaskWins
{
    private readonly object m_pendingToken = new object();
    private Task m_currentTask = Task.FromResult(true); //It will always have a non-null value that you can await on
    private Func<Task> m_nextTaskFunc;

    public async Task<bool> TryRunAsync(Func<Task> taskFunc)
    {
        lock (m_pendingToken)
        {
            //No need in double-checked locking as the last wins
            m_nextTaskFunc = taskFunc;
        }

        // Current task is always present, and may already be completed (in which case await would run synchronously).
        await m_currentTask;

        Task currentTask;

        //Using double-checked locking to make sure we don't have concurrency issues around continuation task, and we only call taskFunc when going to await on it
        if (m_nextTaskFunc != taskFunc)
            return false;

        lock (m_pendingToken)
        {
            if (m_nextTaskFunc != taskFunc)
                return false;

            currentTask = m_currentTask = taskFunc();
            m_nextTaskFunc = null; //just to clean up the reference, as may represent a memory leak otherwise
        }

        await currentTask;
        return true;
    }
}


ASP.NET MVC website that display blogs from the database by AJAX The problem is, I have to limit the results to only 10 by default and add a "Load More" button to display 10 more results each time when user click the button. But I'm worried about users having their JavaScript disabled, because in that case, users won't be able to see more blogs.

But I can also use MVC Razor Syntax codes to display more results by requesting the URL parameters by a button inside a form and it's also works fine and it's only a few lines of codes, but the only problem is the URL parameters showing in the address bar and I don't want them to be showing up.

Now I'm putting my MVC Razor Syntax codes to inside a  tag for JavaScript Backup and it's working perfectly.

What I want to know is there any disadvantage of doing this or if there is any better solution to achieve this. I don't know much about the NoScript tags or how that works with Razor Syntax codes.

.cshtml

    @{
        var rowsNumb = 10;
        var db = Database.Open("Northwind");
        rowsNumb = Request["lastRec"].AsInt() + 10;
        var maxRecs = db.QueryValue("SELECT COUNT([Product Id]) FROM Products");
        var sql = @"SELECT * FROM Products ORDER BY [Product Id] OFFSET 0 ROWS FETCH NEXT @0 ROWS  ONLY";
        rowsNumb = (rowsNumb > maxRecs ? maxRecs : rowsNumb);
        var result = db.Query(sql, rowsNumb);
    }

        @foreach(var row in result){

                @row["Product Name"]
                @row["English Name"]
                @row["Unit Price"]

        }

    <form>
        <input type="hidden" name="lastRec" value="@rowsNumb" />
        <button type="submit">Load More</button>
    </form>

Putting your JavaScript alternative in <noscript> is exactly what <noscript> was meant for. There is nothing in the specs that would make this a bad idea.

I have to note that the derivation of rowsNumb in your code is terrible:

    var rowsNumb = 10;   //<- RowsNumb is 10
    var db = Database.Open("Northwind");
    rowsNumb = Request["lastRec"].AsInt() + 10; //No longer 10, but last request + 10
    var maxRecs = db.QueryValue("SELECT COUNT([Product Id]) FROM Products");
    var sql = @"SELECT * FROM Products ORDER BY [Product Id] OFFSET 0 ROWS FETCH NEXT @0 ROWS  ONLY";
    rowsNumb = (rowsNumb > maxRecs ? maxRecs : rowsNumb); //Possibly not last request + 10 ..
    var result = db.Query(sql, rowsNumb);

You could try something like this:

    var db = Database.Open("Northwind");
    var maxRecs = db.QueryValue("SELECT COUNT([Product Id]) FROM Products");
    var rowsNumb = Math.min( Request["lastRec"].AsInt() + 10 , maxRecs )
    var sql = @"SELECT * FROM Products ORDER BY [Product Id] OFFSET 0 ROWS FETCH NEXT @0 ROWS  ONLY";
    var result = db.Query(sql, rowsNumb);

I am also not 100% about your SQL statement, but given that it goes against Nortwind..


Range check operator I'd like to improve this operator code if I can. I don't know enough Raku
to write idiomatically (or any other way for that matter), but suggestions
along those lines would be good also.

use v6d;

sub circumfix:<α ω>( ( $a, $b, $c ) ) {
    if ( $a.WHAT ~~ (Int) ) {
        so ( $a >= $b & $a <= $c );
    }
    else {
        my @b;
        for $a { @b.push( ( $_ >= $b & $_ <= $c ).so ) };
        so (False) ∈ @b;
    }
};

if (α 5, 0, 10 ω) {
    "Pass".say;
}
else {
    "Fail".say;
}
if (α ( 5, 7, 11 ), 0, 10 ω) {
    "Pass".say;
}
else {
    "Fail".say;
}

This does a range check on a variable or variables, between/bookended by $b (lower) and $c (upper). I wonder about using
some sort of signature check paired with multi as a cleaner
approach?

There is quite a lot that you can do to make this idiomatic. I'll also go a bit overboard and show some ways to fully polish it (emphasis on way overboard for most projects).

You mention potentially wanting to do multi.  That's certainly possible here:

multi sub circumfix:<α ω>( ( $a where Positional, $b, $c ) ) { ... };
multi sub circumfix:<α ω>( ( $a where Int,        $b, $c ) ) { ... };

But ultimately it's unnecessary.  As the other answer points out, Raku has a junction types built in. They can be made by using the routines (or operators)  all (&), any (|), one (^) and none — the routines are available as both subs and methods.  In your case, we can take advantage of the fact that junctions can be made on a single item or a list, so that regardless whether $a is a list of Int or just a single Int, we can make it into a junction.  That right there can reduce your code to:

sub circumfix:<α ω>( ( $a, $b, $c ) ) {
   so ( $a.any >= $b & $a.any <= $c );
};

Although we're going to remove it entirely, it should be noted that & is a junction. This is slightly less efficient than using && which just checks whether both sides are true and evaluates to a Bool.  But we won't need to do that at all.  Raku allows for chained operators, and the comparison operators are defined as such.  This means that we can simplify things even further to:

sub circumfix:<α ω>( ( $a, $b, $c ) ) {
   so $b ≤ $a.any ≤ $c;
};

so has very loose operator precedence, so we don't need parentheses here.  The final two considerations are purely stylistic, but help for code maintaince.  The names $a, $b, and $c aren't descriptive.  You'd definitely want to use something better.  Also, it's fairly common, though not universal, to give extra emphasis to the fact that we're not mutating the values by using sigil-less variables.  Personally, I also like to use whitespace to indicate the tighter grouping when using deconstruction, but YMMV.  Lastly, semicolons aren't needed when they're the final expression of a block, or when a block terminates the line, so we can remove the semicolons.

sub circumfix:<α ω>( (\val,\min,\max) ) {
   so min ≤ val.any ≤ max
}

In this case, using min/max/val without sigils maybe is a bit riskier as they are also routines, but it seems to workout okay.

If we want to go above and beyond for safety's sake, we can check the values in the signature.  I'm assuming based on your original that you wanted to limit things to Int values so let's make it explicit, and specify our return value:

sub circumfix:<α ω>( (\val where .all ~~ Int, \min where Int, \max where Int) --> Bool) {
   so min ≤ val.any ≤ max
}

At this point, the signature is getting a bit unruly, and so I like to spread it across multiple lines.  Particularly if you have several deconstructed arguments, this lets identation also show structure a bit more clearly (but in this case, it's less necessary of course):

sub circumfix:<α ω> (
    (
       \val where .all ~~ Int,
       \min where         Int,
       \max where         Int
    )
    --> Bool
) {
   so min ≤ val.any ≤ max
}

And don't forget that Raku's POD syntax means we can also add in documentation that could help users or automatically create documentation:

#| Determine if a value or values are between the given values (inclusive)
sub circumfix:<α ω> (
    (
       \val where .all ~~ Int, #= The Int value or values to be tested
       \min where         Int, #= The lowest acceptable value
       \max where         Int  #= The highest acceptable value
    )
    --> Bool
) {
   so min ≤ val.any ≤ max
}

IDEs like Comma are already able to pull some of this information to make coding easier.  Using it on signatures is really only supported for generating help feedback automatically on command line usage, but it's only a matter of time before such things will be integrated into IDEs.


Hangman game written in Python 3.5 This collects user's guesses for the word. The user has 8 guesses, a correct guess and a repeated guess does not affect the guess count.

def getAvailableLetters(lettersGuessed):
    '''
    lettersGuessed: list, what letters have been guessed so far
    returns: string, comprised of letters that represents what letters have not
      yet been guessed.
    '''
    import string
    fullstring = string.ascii_lowercase
    lettersLeft = ''
    for letter in fullstring:
        if letter not in lettersGuessed:
            lettersLeft = lettersLeft + letter
    return lettersLeft

def getGuessedWord(secretWord, lettersGuessed):
    '''
    secretWord: string, the word the user is guessing
    lettersGuessed: list, what letters have been guessed so far
    returns: string, comprised of letters and underscores that represents
      what letters in secretWord have been guessed so far.
    '''
    wordGuessed = ''
    for letter in secretWord:
        if letter in lettersGuessed:
            wordGuessed = wordGuessed + letter
        else:
            wordGuessed = wordGuessed + '_ '
    return wordGuessed

def isWordGuessed(secretWord, lettersGuessed):
    '''
    secretWord: string, the word the user is guessing
    lettersGuessed: list, what letters have been guessed so far
    returns: boolean, True if all the letters of secretWord are in lettersGuessed;
      False otherwise
    '''
    numCorrect = 0
    for letter in secretWord:
        if letter in lettersGuessed:
            numCorrect += 1
        else:
            return False
    return True

def hangman(secretWord):
    guessesLeft = 8
    lettersGuessed =[]

    print('Welcome to the game Hangman!')
    print('I am thinking of a word that is ' + str(len(secretWord)) + ' letters long.' )
    print('-----------')

    while guessesLeft > 0:
        if isWordGuessed(secretWord, lettersGuessed):
            return print('Congratulations, you won!')
        print('You have ' + str(guessesLeft) + ' guesses left.')
        print('Available Letters: ' + getAvailableLetters(lettersGuessed))
        user_input = input('Please guess a letter: ')
        user_input = str(user_input)
        user_input = user_input.lower()

        if user_input not in lettersGuessed:
            lettersGuessed.append(user_input)

            if user_input in secretWord:
                print("Good guess: " + getGuessedWord(secretWord, lettersGuessed))
                print('-----------')
            else:
                print("Oops! That letter is not in my word: " + getGuessedWord(secretWord, lettersGuessed))
                print('-----------')
                guessesLeft -= 1
        else:
            print("Oops! You've already guessed that letter: " + getGuessedWord(secretWord, lettersGuessed))
            print('-----------')

    return print("Sorry, you ran out of guesses. The word was " + str(secretWord))

hangman('stackoverflow')

Multiple PEP8 Style Violations

There's a bunch of style violations here, and it really irks me.  You are also violating fundamental parts of Python syntax with some of these things, especially with indentation, so you should try and correct style issues as well as my other concerns/suggestions.  I try not harping on style, but there's enough issues here that it irks me enough to put this first.

import statements must be at the beginning of the file.
Function definitions need to be padded by two line breaks.  That is, from the end fo one function to the start of the next, you need two blank lines between them (except inside a Class definition, which you don't have here).
Function names should be all lowercase, and multiple-word names for functions should be lowercase and separated with underscores (getGuessedWord should be get_guessed_word, etc.)
Variable names should be all lowercase, not CamelCase.  (The exception are module-wide constants, which should be in all CAPS.).  Multi-word variable names (such as lettersGuessed) should be lowercase and separated with underscores (letters_guessed)
Docstrings should be triple-quoted, not triple-apostrophe'd.  (""", not ''')
You have indentation problems that make your code error out in certain cases under hangman where you will have unexpected indentation. (note that I suppress these when testing code here for review, but it's a critical issue).  Use 100% consistent indentation - this should be 4 spaces per indentation level, not tabs, and you should not have some lines indented where they are not supposed to be.

User input can be condensed to one line

You currently use this for getting user input:

user_input = input('Please guess a letter: ')
user_input = str(user_input)
user_input = user_input.lower()

This can be condensed into one line, and we can also trim off the line break at the end too (so later recommendations for validation work without much issue):

user_input = input('Please guess a letter: ').lower().strip('\r\n')

No checking for valid inputs

You are expecting only letters to be provided, correct?  That's the basis of Hangman.

We should probably test to see if we're actually getting letters provided to us, and make a warning.  So, let's do this right after the user enters input:

user_input = input('Please guess a letter: ')).lower().strip('\r\n')
while user_input not in string.ascii_lowercase:
    print("You have not provided a letter, please only use letters!")
    user_input = input('Please guess a letter: ').lower().strip('\r\n')

Output messages: Code repetition

You've got this in here three times:

print('-----------')

You can instead use only one of these, and place it after the if/else block, and thereby not have to repeat this three times in a row inside the if block (note that I've snipped out all other contents of the if/else block, to shorten this post slightly).

if user_input not in letters_guessed:
    ....
else:
    ....

print('-----------')

Use augmented assignment statements.

This is currently in your code in getGuessedWord:

if letter in letters_guessed:
    word_guessed = word_guessed + letter
else:
    word_guessed = word_guessed + '_ '

Two things here: both of these can be condensed using augmented assignment statements, and you may wish to just use underscores without extra spaces between characters in the word, because it looks messy when I have some letters guessed and others not guessed (inconsistent spacing in output):

if letter in letters_guessed:
    word_guessed += letter
else:
    word_guessed += '_'

Use if __name__ == "__main__": to define what to execute when script is called from command line

python3 hangman.py will call hangman("stackoverflow"), but this is not really a good way to achieve it.

We should primarily define something like this at the end of the program to make sure that what runs is what we want to run when this is executed at the command line:

if __name__ == "__main__":
    hangman('stackoverflow')

Note though that you are always going to have "stackoverflow" as the string; you should strongly consider providing a file with valid words for usage, and then random selection therein rather than using a static string as an assignment.

  This has an added benefit of 'guarding' the call to hangman() (which is the replacement for a call to main() as some would have) if, for some reason, I was importing parts of the module and importing into another one.  This is more or less, also, a personal preference to guard whether I plan on importing or not, it's something I do regardless, just to protect calls to 'start' the program/script.

Unnecessary creation of variables

In get_available_letters, you do this:

fullstring = string.ascii_lowercase
....
for letter in fullstring:

Why do we create fullstring here?  It's not needed - we can just use this instead and not have to create the fullstring variable:

for letter in string.ascii_lowercase:


Secure Token for use in API calls My team has developed an API and I've been tasked with creating an authentication layer that allows partners' applications to consume it. To that end, I've created a data store which houses an Application ID, Login and salted/hashed Password for that application to authenticate against. I want to hand back a Token for them to use (which is a key for session information that is kept on the server). The process to generate the Token is below. I have another process which reverses this to validate its authenticity. It seems to stand up to testing, but I'd like to know if there's anything else that should be done to make this properly secure.

private const string Password = "nottheactualpasswordthatisused..";

private static readonly byte[] _Salt =
{
    0x00, 0x01, 0x02, 0x03,
    0x04, 0x05, 0x06, 0x07,
    0x08, 0x09, 0x0a, 0x0b,
    0x0c, 0x0d, 0x0e, 0x0f
};

private static async Task<string> CreateSecureSessionTokenAsync(int applicationId, DateTime createDate)
{
    using (var aes = Aes.Create())
    {
        if (aes == null)
        {
            return null;
        }

        using (var deriveBytes = new Rfc2898DeriveBytes(Password, _Salt))
        {
            aes.Key = deriveBytes.GetBytes(32);
            aes.IV = deriveBytes.GetBytes(16);
        }

        using (var encryptor = aes.CreateEncryptor())
        using (var memoryStream = new MemoryStream())
        {
            using (var cryptoStream = new CryptoStream(memoryStream, encryptor, CryptoStreamMode.Write))
            using (var writer = new StreamWriter(cryptoStream))
            {
                await writer.WriteLineAsync(Convert.ToString(applicationId)).ConfigureAwait(false);
                await writer.WriteLineAsync(Convert.ToString(createDate, CultureInfo.InvariantCulture))
                    .ConfigureAwait(false);
            }

            return Convert.ToBase64String(memoryStream.ToArray());
        }
    }
}

Using a string for storing a password is considered insecure because there is no possibility to delete that string from memory. From the SecureString documentation

  An instance of the System.String class is both immutable and, when no longer needed, cannot be programmatically scheduled for garbage collection; that is, the instance is read-only after it is created, and it is not possible to predict when the instance will be deleted from computer memory. Because System.String instances are immutable, operations that appear to modify an existing instance actually create a copy of it to manipulate. Consequently, if a String object contains sensitive information such as a password, credit card number, or personal data, there is a risk the information could be revealed after it is used because your application cannot delete the data from computer memory.

so it would be better to use a SecureString but unfortunately that isn't an option when using Aes because it doesn't take an SecureString as a parameter. So what other options are there ? Just take a char[] and overwrite its content when the class is disposed.

The CreateSecureSessionTokenAsync() method should ideally return the token along with a random generated salt (maybe as tuple) to be stored in the data store to decrypt it later without the need that the customer will get the salt together with the token.

An implementation of a random salt using a parameterized length can be found in my answer here.


Generic repository with UoW and simple injector I'm implementing a generic repository + UoW pattern with EF6 and simple injector. I feel there is room for improvements. Please let me know if I left out some relevant code, I tried to keep it breif.

Do you see any need for the unit of work to keep track of the repositories (in its _repositories field)? As you can see I'm currently not using it.

Another important thing here is that I want one EF context, LGTContext, per web request. But as far as I understand, it would be the case due to the Lifestyle.Scoped registration lifestyle.

Any other thoughts?

Generic repository:

public abstract class DatabaseRepository<T> : IRepository<T> where T : BaseEntity
{
    protected readonly DbSet<T> DbSet;
    protected readonly LGTContext Context;

    protected DatabaseRepository(IUnitOfWork unitOfWork, LGTContext context)
    {
        Context = context;
        DbSet = Context.Set<T>();

        unitOfWork.Register(this);
    }

    public Task<PagedResult<T>> Find(Expression<Func<T, bool>> predicate, int pageNumber, int pageSize)
    {
        throw new NotImplementedException();
    }

    public virtual async Task<IEnumerable<T>> Find(Expression<Func<T, bool>> predicate)
    {
        return await DbSet.Where(predicate).ToListAsync();
    }

    public virtual async Task<IEnumerable<T>> GetAll()
    {
        return await DbSet.ToListAsync();
    }

    public virtual async Task<T> GetSingle(object id)
    {
        return await DbSet.FindAsync(id);
    }

    public virtual void Add(T entity)
    {
        DbSet.Add(entity);
    }

    public virtual void Delete(T entity)
    {
        if (Context.Entry(entity).State == EntityState.Detached)
        {
            DbSet.Attach(entity);
        }
        DbSet.Remove(entity);
    }

    public virtual void Edit(T entity)
    {
        DbSet.Attach(entity);
        Context.Entry(entity).State = EntityState.Modified;
    }
}

Unit of work:

public class UnitOfWork : IUnitOfWork, IDisposable
{
    private readonly Dictionary<Type, IRepository> _repositories = new Dictionary<Type, IRepository>();
    private readonly LGTContext _context;
    private bool _disposed;

    public UnitOfWork(LGTContext context)
    {
        _context = context;
    }

    public async Task CommitAsync()
    {
        try
        {
            await _context.SaveChangesAsync();
        }
        catch (DbEntityValidationException e)
        {
            // todo: handle validation errors...
            throw;
        }
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    protected virtual void Dispose(bool disposing)
    {
        if (!_disposed)
        {
            if (disposing)
            {
                _context.Dispose();
            }
        }
        _disposed = true;
    }

    public void Register(IRepository repository)
    {
        _repositories.Add(repository.GetType(), repository);
    }
}

Simple injector registration:

container.Register<IUnitOfWork, UnitOfWork>(Lifestyle.Scoped);
container.Register(typeof(IRepository<>),
                        new[] { typeof(DatabaseRepository<>).Assembly }, Lifestyle.Scoped);
container.Register<LGTContext>(Lifestyle.Scoped);

Usage psuedo code:

public SomeController(IUnitOfWork uow, IRepository<SomeEntity> repo)
{
    var entities = await repo.GetAll();
    // modify some entities...
    await uow.CommitAsync();
}

Putting aside the fact that EF already provides both a UoW and Generic Repository, there's still a huge flaw with your UnitOfWork. It disposes a resource it doesn't own. You pass the context in as a constructor arg, store it as a member, and then code analysis yelled at you for not implementing IDisposable, so you disposed of it. The problem is, the UoW has no way of knowing that it's supposed to dispose of the context, because it was injected into the class.

In all the years I've been working with .Net, not once have I actually needed to implement IDisposable. Every single time that I thought I needed to, I didn't. I just needed to keep the lifetime of the IDisposable short and contained within a using block. That's possible if you abandon this folly and implement a domain specific service instead.

public class EFWidgetStore : IWidgetStore
{
    private readonly Func<DbContext> GetContext;
    public EFWidgetStore(Func<DbContext> contextFactory)
    {
        GetContext = contextFactory;
    }

    public Widget Get(int id)
    {
        using (context = GetContext())
        {
            //... get it
        }
    }

    public Widget Get(string name)
    {
         //...
    }

Next up, you've broken Liskov's substitution principle, which is common for these kinds of implementations.

public Task<PagedResult<T>> Find(Expression<Func<T, bool>> predicate, int pageNumber, int pageSize)
{
    throw new NotImplementedException();
}

Although, let's be honest. When will you ever treat a specific repository as an instance of the base class anyway... You've introduced a world of hurt on yourself for no gain what so ever. Please don't do this.


Comparing products sold by several merchants My application is for the use of comparing a product sold by several merchants on a daily basis. A process runs to load the current values of data for each store and enters it into a database along with any previous data. Later I plan to create graphs and such off of the history of data.

SELECT `a`.`part_number`, GROUP_CONCAT(`a`.`seller_id` ORDER BY `price` ASC), GROUP_CONCAT(`b`.`part_id` ORDER BY `price` ASC), GROUP_CONCAT(`b`.`price` ORDER BY `price` ASC)
FROM `parts`
LEFT JOIN (
           SELECT *
           FROM (
                 SELECT *
                 FROM `parts_data` AS `tmp_a`
                 ORDER BY `date_added` DESC
                ) AS `tmp_b`
           GROUP BY `part_id`
          ) AS `b`
ON `a`.`part_id` = `b`.`part_id`
GROUP BY `a`.`part_number`

To explain the query a little, I need to select the most recent price information for several part IDs and group them together with a selected part number for comparison. Also, upon concatenation the grouped items are sorted by price to be highest to lowest.

Each part number also can have any number of item IDs associated with it so there is no implication of how many companies are being watched.

part_number |  column1  |  column2  |  column3
++++++++++++++++++++++++++++++++++++++++++++++++
            | seller:A1 | seller:A3 | seller:A6
  153231    | id:x423   | id:x534   | id:x902
            | cost:23   | cost:34   | cost:57
++++++++++++++++++++++++++++++++++++++++++++++++
            | seller:A3 | seller:A1 | seller:A6
  345123    | id:x313   | id:x631   | id:x652
            | cost:78   | cost:86   | cost:99
++++++++++++++++++++++++++++++++++++++++++++++++
            | seller:A5 | seller:A1 |
  231756    | id:x663   | id:x291   |
            | cost:35   | cost:52   |

The ORDER BY inside the inner-most sub-select is pointless - it's going to get thrown out.  Also, you should never rely on MySQL's behaviour of selecting a 'random' value for non-grouped columns - always explicitly specify the transformation that needs to happen on the data.  You should also review what you think your Company columns are, as I am going to pretty much guarantee that the company isn't the same for every cell in the column.

EDIT:

Actually, I was thinking your ORDER BY in the GROUP_CONCAT() was just fine - I have issues with how you're doing your inner-most SELECT... ORDER BY (that you then do a GROUP BY off of, without specifying your transforms).
Assuming that the tuple [parts_data.part_id, parts_data.date] is unique (although, that's the date of what?  Insert, update, your birthday?  Please name more descriptively), here's how I would have structured this (in a system without CTEs and OLAPs):

SELECT `a`.`part_number`, GROUP_CONCAT(`a`.`seller_id` ORDER BY `b`.`price` ASC),
                          GROUP_CONCAT(`b`.`part_id` ORDER BY `b`.`price` ASC),
                          GROUP_CONCAT(`b`.`price` ORDER BY `b`.`price` ASC)
FROM `parts` as `a`
LEFT JOIN (`parts_data` as `b`
           INNER JOIN (SELECT `part_id`, MAX(`date`) AS `date`
                       FROM `parts_data`
                       GROUP BY `part_id`) as `c`
           ON `b`.`part_id` = `c`.`part_id`
           AND `b`.`date` = `c`.`date`)
ON `a`.`part_id` = `b`.`part_id`
GROUP BY `a`.`part_number`
ORDER BY `a`.`part_number`

Please note that I do not in fact have a MySQL instance to test against.  I also don't know if this will be faster for your database (please note we now have a self-join).  However, it's much clearer what you're expecting the system to do, meaning it's easier to maintain.

EDIT:

Here's an alternate version, which may be slightly faster.  I don't work with MySQL (and I have some fairly serious hardward backing me up on DB2), so I'm not up on all the ins and outs.

SELECT `a`.`part_number`, GROUP_CONCAT(`a`.`seller_id` ORDER BY `b`.`price` ASC),
                          GROUP_CONCAT(`b`.`part_id` ORDER BY `b`.`price` ASC),
                          GROUP_CONCAT(`b`.`price` ORDER BY `b`.`price` ASC)
FROM `parts` as `a`
LEFT JOIN (SELECT `b`.`part_id`, `b`.`price`
           FROM `parts_data` as `b`
           WHERE NOT EXISTS (SELECT '1'
                             FROM `parts_data` as `c`
                             WHERE `c`.`part_id` = `b`.`part_id`
                             AND `c`.`date` > `b`.`date`)) as `b`
ON `b`.`part_id` = `a`.`part_id`
GROUP BY `a`.`part_number`
ORDER BY `a`.`part_number`

You may want to try removing the ORDER BY a.part_number clause, and see if that helps.


Buffer concatenation method Is there any way to improve or optimize the speed of this? I can't think of any ways to do it in one pass.

static byte[] ConcatenateBuffers(params byte[][] list)
{
    int totalLen = 0;
    for (int i = 0; i < list.Length; ++i)
        totalLen += list[i].Length;
    byte[] ret = new byte[totalLen];
    int offset = 0;
    for (int i = 0; i < list.Length; ++i)
    {
        Array.ConstrainedCopy(list[i], 0, ret, offset, list[i].Length);
        offset += list[i].Length;
    }
    return ret;
}

If you're optimizing for performance it does not get much better than that. In particular it is a valuable optimization to compute the exact result array length.

I'm not sure why you used ConstrainedCopy which is a very esoteric feature. Copy is faster. ConstrainedCopy is, I believe, used for code that must not be interrupted by a ThreadAbortException. The concept is called a constrained execution region and it's useless in practice because programming in the presence of thread aborts is essentially impossible. The consensus is to never abort a thread except your own (ASP.NET violates that rule which must be very unsafe).

The code is of good quality as well. I'd use foreach to traverse the list. It compiles to the same code that you wrote but is nicer.

Answers have been proposed that use List<byte> or LINQ. These will be far slower (e.g. 10-100x). This is because Array.Copy is a facade over memmove (which is similar to memcpy but it handles overlapping regions as well). The difference becomes noticeable even with common network speeds (10MB/s) and certainly with disk speeds (100MB/s and up).

If you don't care about performance or the amount of data processed is small than you can use slower approaches. Some people find such code better and I'd certainly understand why. The LINQ approach using Concat is extremely concise and self-documenting.


A tiny module for handling text updates I call these text updates "Feeds". A Feed object has some basic attributes like its content string, how many lives it has and its priority in the queue. I haven't made a unittest for this one, but below I have added a small test function for the purpose of posting this question. I use this to update statusbar in GUI applications.

import threading
import queue
import time

LOW_PRIORITY = 0
MEDIUM_PRIORITY = 1
HIGH_PRIORITY = 2

class Feed(object):
    def __init__(self, msg, life=1, priority=LOW_PRIORITY):
        self.msg = msg
        self.life = life
        self.priority = LOW_PRIORITY if life == -1 else priority

    def hit(self):
        if self.life > 0:
            self.life -= 1
        elif not self.life:
            raise ValueError("Feed {} is already dead.".format(self))

    def __bool__(self):
        return True if self.life else False

    def __lt__(self, obj):
        if self.priority > obj.priority:
            return True
        return False

class FeedThread(threading.Thread):
    def __init__(self, globevent, handler, updateinterval=3):
        super(FeedThread, self).__init__()

        self.globevent = globevent
        self.handler = handler
        self.updateinterval = updateinterval
        self.feedqueue = queue.PriorityQueue()

    def PushFeed(self, feed):
        self.feedqueue.put(feed)

    def run(self):
        while not self.globevent.is_set():
            try:
                feed = self.feedqueue.get(block=True, timeout=1)
                if feed:
                    self.handler(feed.msg)
                    feed.hit()
                    self.PushFeed(feed)
                    time.sleep(self.updateinterval)
            except queue.Empty:
                pass

def test_feed():
    newstime = threading.Event()
    def bullettin(news):
        print("\r{:^79}".format(news), end="")
    scroll = FeedThread(newstime, bullettin)
    scroll.start()
    sports_feed = Feed("Barclays Premier League - Man Utd V Tottenham - 12:45",
                        life=2, priority=MEDIUM_PRIORITY)
    weather_feed = Feed("London - Cloudy, High 26C. Winds EDE at 15 to 30 km/h",
                        life=-1)
    entertainment_feed = Feed("Upcoming movie - Pixels (2015)",
                            life=2)
    for feed in (sports_feed, weather_feed, entertainment_feed):
        scroll.PushFeed(feed)
    time.sleep(15)
    breaking_feed = Feed("BREAKING NEWS - Cats love lasers - pew pew pew!",
                        priority=HIGH_PRIORITY)
    scroll.PushFeed(breaking_feed)
    time.sleep(15)
    newstime.set()

if __name__ == "__main__":
    test_feed()

A few quick comments:

In __bool__(), rather than using an if statement, just do return bool(self.life). It has the same effect, but it's much cleaner.
Likewise, in __lt__(), you can just do return (self.priority > obj.priority).

Note that the convention in those comparison scenarios is to call the other object other, not obj, so I would recommend renaming the variable.
There are no docstrings or comments, which is going to make life very difficult for anybody reading the code later. You should explain what the code is supposed to be doing, and why – right now it’s just guesswork.


Is this a good way to download images from json Api? In my SwiftUI app, I'm trying to get some data from a json Api and display this data along with an image. First I have a function that fetches the data from the Api, then I have another function that downloads the image data. The image takes a while to appear(1-2) seconds.
I'm wondering if this is the best way to do it, especially if I'm downloading multiple images to display them in a List or a Grid.
func fetchAPI() {

    URLSession.shared.dataTask(with: url) { (data, response, error) in

/* Code that decodes the json and gets the imageURL string */

            DispatchQueue.main.async {

                self.downloadImage(url: imageURL) // call the function that downloads the imageData
            }
        } catch {
            print("Error: \(error.localizedDescription)")
        }
    }
    .resume()
}

The function that downloads the image:
func downloadImage(url: String) {

    guard let imageURL = URL(string: url) else {
        print("Error: Invalid image URL")
        return
    }

    URLSession.shared.dataTask(with: imageURL) { data, response, error in
        guard let recievedData = data, error == nil  else {
            print("Error: No Data")
            return
        }

        DispatchQueue.main.async {
            self.imageData = recievedData
        }
    }
    .resume()
}

Maybe the image is in question is a little bit large or the internet connection used is a little bit slow.
I personally would go about it differently.
For UIKit
import UIKit
import Foundation

extension UIImageView {
    func load(url: URL) {
        DispatchQueue.global().async { [weak self] in
            if let data = try? Data(contentsOf: url) {
                if let image = UIImage(data: data) {
                    DispatchQueue.main.async {
                        self?.image = image
                    }
                }
            }
        }
    }
}

For SwiftUI we have to go about it a little bit differently but there are two ways
The very simple one-time usage:
import Foundation
import SwiftUI
import Combine

// Image Loader
class ImageLoader: ObservableObject {
    var dataPublisher = PassthroughSubject<Data, Never>()
    var data = Data() {
        didSet {
            dataPublisher.send(data)
        }
     }

     init(urlString:String) {
        guard let url = URL(string: urlString) else { return }
        let task = URLSession.shared.dataTask(with: url) { data, response, error in
            guard let data = data else { return }
            DispatchQueue.main.async {
                self.data = data
            }
        }
        task.resume()
     }
}

// Image View
struct ImageView: View {
    @ObservedObject var imageLoader:ImageLoader
    @State var image:UIImage = UIImage()

    init(withURL url:String) {
        imageLoader = ImageLoader(urlString: url)
    }

    var body: some View {
        VStack {
            Image(uiImage: image)
                .resizable()
                .aspectRatio(contentMode: .fit)
                .frame(width:100, height:100)
        }.onReceive(imageLoader.dataPublisher) { data in
            self.image = UIImage(data: data) ?? UIImage()
        }
    }
}

struct ImageView_Previews: PreviewProvider {
    static var previews: some View {
        ImageView(withURL: "")
    }
}

The more advanced way
import Foundation
import SwiftUI
import Combine

// ImageCache
protocol ImageCache {
    subscript(_ url: URL) -> UIImage? { get set }
}

struct TemporaryImageCache: ImageCache {
    private let cache: NSCache<NSURL, UIImage> = {
        let cache = NSCache<NSURL, UIImage>()
        cache.countLimit = 100 // 100 items
        cache.totalCostLimit = 1024 * 1024 * 100 // 100 MB
        return cache
    }()

    subscript(_ key: URL) -> UIImage? {
        get {
            self.cache.object(forKey: key as NSURL)
        }
        set {
            newValue == nil ? self.cache.removeObject(forKey: key as NSURL) : self.cache.setObject(newValue!, forKey: key as NSURL)
        }
    }
}

// EnvironmentValues+ImageCache
struct ImageCacheKey: EnvironmentKey {
    static let defaultValue: ImageCache = TemporaryImageCache()
}

extension EnvironmentValues {
    var imageCache: ImageCache {
        get {
            self[ImageCacheKey.self]
        }
        set {
            self[ImageCacheKey.self] = newValue
        }
    }
}

// Image Loader
class ImageLoader: ObservableObject {
    @Published
    var image: UIImage?

    private(set) var isLoading = false

    private let url: URL
    private var cache: ImageCache?
    private var cancellable: AnyCancellable?

    private static let imageDownloadingQueue = DispatchQueue(label: "image-downloading")

    init(url: URL, cache: ImageCache? = nil) {
        self.url = url
        self.cache = cache
    }

    deinit {
        self.cancel()
    }

    func load() {
        guard !self.isLoading else { return }

        if let image = self.cache?[self.url] {
            self.image = image
            return
        }

        self.cancellable = URLSession.shared.dataTaskPublisher(for: self.url)
            .map {
                UIImage(data: $0.data)
            }
            .replaceError(with: nil)
            .handleEvents(receiveSubscription: { [weak self] _ in
                self?.onStart()
            },
            receiveOutput: { [weak self] in
                self?.cache($0)
            },
            receiveCompletion: { [weak self] _ in
                self?.onFinish()
            },
            receiveCancel: { [weak self] in
                self?.onFinish()
            })
            .subscribe(on: Self.imageDownloadingQueue)
            .receive(on: DispatchQueue.main)
            .sink { [weak self] in
                self?.image = $0
            }
    }

    func cancel() {
        self.cancellable?.cancel()
    }

    private func onStart() {
        self.isLoading = true
    }

    private func onFinish() {
        self.isLoading = false
    }

    private func cache(_ image: UIImage?) {
        image.map {
            self.cache?[self.url] = $0
        }
    }
}

// Image View
struct ImageView<Placeholder: View>: View {
    @StateObject
    private var loader: ImageLoader
    private let placeholder: Placeholder
    private let image: (UIImage) -> Image

    init(url: URL, @ViewBuilder placeholder: () -> Placeholder, @ViewBuilder image: @escaping (UIImage) -> Image = Image.init(uiImage:)) {
        self.placeholder = placeholder()
        self.image = image
        self._loader = StateObject(wrappedValue: ImageLoader(url: url, cache: Environment(\.imageCache).wrappedValue))
    }

    var body: some View {
        self.content.onAppear(perform: loader.load)
    }

    private var content: some View {
        Group {
            if let image = self.loader.image {
                self.image(image)
            } else {
                self.placeholder
            }
        }
    }
}

```


Recursion vs. iteration in Lisp macro I've been programming Clojure for a little while and recently started learning Common Lisp. One of my favorite things about Clojure is the threading operator ->, which greatly simplifies long chains of nested function calls. Naturally I wanted to have this in Common Lisp.

I found an implementation here:

(defmacro -> (x &rest args)
  (destructuring-bind (form &rest more)
      args
    (cond
      (more `(-> (-> ,x ,form) ,@more))
      ((and (consp form)
            (or (eq (car form) 'lambda)
                (eq (car form) 'function)))
       `(funcall ,form ,x))
      ((consp form) `(,(car form) ,x ,@(cdr form)))
      (form `(,form ,x))
      (t x))))

This uses a recursive macro expansion; I've read that it's better to use iteration over recursion in CL when you can, so I wrote my own version:

(defmacro -> (x &rest forms)
  (labels ((expand-form (x form)
             (if (consp form)
                 (if (or (eq (car form) 'lambda)
                         (eq (car form) 'function))
                     `(funcall ,form ,x)
                     `(,(car form) ,x ,@(cdr form)))
                 `(,form ,x))))
    (do ((forms forms (cdr forms))
         (x x (expand-form x (car forms))))
        ((not forms) x))))

I'm relatively new to Lisp so I don't know how to judge which way is better. Any comments, suggestions?

EDIT: I knew something was fishy! You don't have to use any looping constructs at all, a plain old reduce will cut it:

(defmacro -> (x &rest forms)
  (flet ((expand-form (x form)
           (cond
             ((atom form)
              `(,form ,x))
             ((member (car form) '(lambda function))
              `(funcall ,form ,x))
             (t
              `(,(car form) ,x ,@(cdr form))))))
    (reduce #'expand-form forms :initial-value x)))

I think the first version is slightly more elegant and looks okay to me.

In the second version, I would replace LABELS with FLET. LABELS would indicate some kind of recursion.

Generally I would also think if the nested expansion is something I would want, vs. an expansion into a LET* form.

I would also replace CAR and CDR with FIRST and REST.

The DESTRUCTURING-BIND is also not necessary:

(defmacro -> (x form &rest more)
  (cond (more
         `(-> (-> ,x ,form) ,@more))
        ((and (consp form)
              (member (first form) '(lambda function)))
         `(funcall ,form ,x))
        ((consp form)
         `(,(first form) ,x ,@(rest form)))
        (form
         `(,form ,x))
        (t x)))


Looping through multiple regex efficiently I'm writing a program that adds all network interfaces with multiple parameters to a list.

I have written the following code, which should work out of the box on any Linux system with Python and ethtool installed:

import imp
import os
import subprocess
import re
from enum import Enum

class interface_type(Enum):

    OPTIC = 1
    TWISTED_PAIR = 2

class NetworkInterface:

    def set_indentifier(self, id):
        self.id = id

    def set_mac_address(self, mac):
        self.mac = mac

    def set_interface_type(self, interface_type):
        self.interface_type = interface_type

def findNetworkInterfaces(ignoredInterface):

    filteredList = netifaces.interfaces()
    network_interfaces = []

    for interface in filteredList:
        for regex in ignoredInterface:
            if re.search(regex, interface):
                break
            else:
                nwi = NetworkInterface()
                nwi.set_indentifier(interface)
                nwi.set_mac_address(setMAC(interface))
                nwi.set_interface_type(setInterfaceType(interface))
                network_interfaces.append(nwi)
                filteredList.remove(interface)
                break
    return network_interfaces

def setMAC(identifier):

    addrs = netifaces.ifaddresses(identifier)
    mac_address = addrs[netifaces.AF_LINK][0].get("addr")

    return mac_address

def setInterfaceType(identifier):

    bashCommand1 = "ethtool " + identifier
    bashCommand2 = "grep ports"

    try:
        process1 = subprocess.Popen(bashCommand1.split(),
            stdout=subprocess.PIPE)
        process2 = subprocess.Popen(bashCommand2.split(),
            stdin=process1.stdout, stdout=subprocess.PIPE)
        output, error = process2.communicate()

    except:
        print ("Error determining interface type: " + error + "\n")
        print ("Interface will be treated as Optical \n")

    if "TP" in output:
        return interface_type.TWISTED_PAIR
    else:
        return interface_type.OPTIC

if __name__ == "__main__":

    ignored_interface = ["lo", "wlp2s0"]

    try:
        imp.find_module("netifaces")
        netifaces_status = True
        import netifaces

    except ImportError as e:
        print ("netifaces not found: " + str(e))
        os.sys.exit(-1)

    network_identifiers = findNetworkInterfaces(ignored_interface)

    #print network_identifiers

    for iface in network_identifiers:
        print iface.id
        print iface.mac
        print iface.interface_type

I'm mainly concerned with the findNetworkInterfaces function, as I feel I'm doing this in a very ineffecient way (basically copying the list and removing interfaces as to not have doubles). In this special case, I'm not concerned with PEP8 - this is something I'll do later. Any other suggestions to improve the code are very welcome.

Start writing in python3.x ;)

End of Life for python2.x will happen pretty soon 1
One regex to rule them all

Instead of looping over a the list to create a regex,

you could use the | char which will work as an or so it can handle multiple outputs

REGEX_FILTER = re.conmpile('lo|wlp2s0')
If you only need the parameters, a simple namedtuple will suffice

You could create a namedtuple that will get rid of the empty looking class
IMHO setting the variables outside of the init is bad style

I would get the different variables needed before, and only then create the NetworkInterface object.

Note I will not review getting the interface type, since I am nowhere near a linux atm

Code

from collections import namedtuple
import re

import netifaces

FILTER_REGEX = re.compile(r'lo|wlp2s0')
NetworkInterface = namedtuple('NetworkInterface', ['iface', 'mac', 'type'])

def get_mac(iface):
    addresses = netifaces.ifaddresses(iface)
    return addresses[netifaces.AF_LINK][0].get("addr")

def get_type(iface):
    """Just return TP for testing"""
    return "TP"

def find_network_interfaces():
    for iface in netifaces.interfaces():
        if FILTER_REGEX.search(iface):
            continue
        yield NetworkInterface(iface, get_mac(iface), get_type(iface))

if __name__ == '__main__':
    for nwi in find_network_interfaces():
        print(nwi.iface)
        print(nwi.mac)
        print(nwi.type)


Compute variance via functional programming So I'm supposed to calculate variance based on this forumula through functional programming:
$$\sigma^2 = \frac{\sum\limits_{i=0}^{n-1} (X -\mu)^2}{n-1}$$
This is the code that works:
public static double variance1(int[] array)
{
     double mean=mean(array);
     double summation=DoubleStream.iterate(0,idx->idx+1)
                                  .limit(array.length)
                                  .reduce(0,(summation_counter,idx)->
                                   {
                                     return Math.pow(array[(int)idx]-mean, 2)+summation_counter;
                                   });

     return summation/(array.length-1);
}

Although the code works, it's ugly with the typecasts and seems forced.
Initially, I thought of doing something like
double summation=IntStream.rangeClosed(0,array.length-1)
                          .reduce(0, (summation_counter,idx)-> ....

but I couldn't use an IntStream as the body of the reduce method returns double.

First off, watch your formating. Java standard is to have spaces around operators, and a space after commas. In the reduce lambda you can leave out the return and the braces, just like you did't for iterate.
It's a bit ugly to use a DoubleStream for the array indices, and number streams provide a sum() method, so using reduce is unnecessary.
An array can easily streamed using Arrays.stream(), which produces an IntStream and  with mapToDouble a DoubleStream can be created:
double summation = Arrays.stream(array)
                     .mapToDouble(x -> (x - mean) * (x - mean))
                     .sum();


Tracking current during iteration I have a list of posts. I need to generate it each one by one. For generation of each post, I need to know about other posts. However, I want to make the current post marked.

def generate(posts):
    print(posts) #faking generation

def marked_active_post(orig_posts, active_index):
    posts_view = orig_posts.copy()
    active_post = orig_posts[active_index]
    active_post = orig_posts[active_index].copy()
    active_post['active?'] = True
    posts_view[active_index] = active_post
    return posts_view

posts = [{'foo':'bar'}, {'aaa':'bbb'}]
for i in range(0, len(posts)):
    generate(marked_active_post(posts, i))

How can I do better?

It's too bad your template engine doesn't support conditions. What about this?

def marked_active_post(orig_posts, active_index):
    posts_view = orig_posts.deepcopy()
    posts_view[active_index]['active?'] = True
    return posts_view

That means n+1 copies instead of 2, but no need to optimize it until it proves to be slow.


Bubble sort optimization This bubble sort code is very slow compared to the original.

import time
import random
def bubble_sort(a):
    #setting my variables
    yes=0
    d=0
    #making a switch
    while yes==0:
        if d>=last_thing-1:
            d=0
        if a[0+d]>a[1+d]:

            #switching the order
            a[0+d],a[1+d]=a[1+d],a[0+d]

        d+=1
        if correct_order(a):
            #turning off the switch
            yes=1

    return(a)
def correct_order(a):
    for i in range(len(a)):
        if i != a[i]:
            return(False)
    return(True)
if __name__ == '__main__':
    #setting your range
    last_thing=1000+1
    #making the random list
    a= list(range(0,last_thing))
    random.shuffle(a)

    #easier setting
    currentsortingmethod=bubble_sort(a)
    #prints your sorted list
    print("--------------------------------------------------------------------------------\n",str(currentsortingmethod).strip('[]').replace(" ", "").replace("'",""))

last_thing
You define last_thing and use it to initialise your array, which is fine, however you also use it within your actual bubble_sort function.  This seems wrong, it would be better to use the length of the supplied array in order to determine when to stop.  This will make your code more portable and able to handle different sized arrays.
When to check for correct_order
You only need to check if the array is sorted at the end of each pass.  As it stands, you're checking if the array is sorted for each entry you're processing, even if you haven't done anything with the array.  Which feels like you're turning an O(n^2) algorithm into an O(n^3) solution.  Another approach you can use is to keep track of whether or not you've actually done a swap on a given pass, if you haven't then you know it's because the list is already sorted.
looping
This is probably very subjective, but I don't like this:
if d>=last_thing-1:
    d=0

To me, it makes the start of each pass through the list less obvious than other methods.
Generalisability
Algorithms are used to solve general problems.  There are several instances where you've tied your solution to the exact problem (such as your use of last_thing) you're solving which means it can't solve the general problem. The way you've implemented correct_order is another good example of this.  You iterate through the list and make sure that each item in the list has the same value as it's position in the list.  This works for your exact problem, but makes your algorithm useless if you wanted to sort [8,2,4,6].  Consider writing unit tests so that you can actually exercise your code with different inputs to make sure they can be handled.  This will help you to build code that is not so tightly coupled to the specific problem you're solving.
Odds and Bobs

You've imported time, but you're not using it.  Redundant code adds unnecessary noise, consider removing the unused import.
Names matter. a and d aren't very descriptive, consider expanding the names to describe what it is they represent.
If you just need two values (0, 1), consider using a boolean instead (False/True).
Try to standardise on your spacing, sometimes you have spaces around operators sometimes you don't... it can make the code look messy.

Taking some of the above into account, a refactored version of your code might look something like this:
def bubble_sort(array):
    while not correct_order(array):
        for index in range(len(array) - 1):
            if array[0+index] > array[1+index]:
                array[0+index],array[1+index] = array[1+index],array[0+index]
    return(array)

def correct_order(array):
    for i in range(len(array)-1):
        if array[i] > array[i+1]:
            return(False)
    return(True)


Validating e-mail address input in C I have this little function to validate e-mail. It's a little piece of code to search major mistakes in e-mail input and it's part of a bigger program I have to complete as a school assignment. Is there a better way to do it in C?

I'd like to do it in regex but if I have understood correctly,  C under Windows doesn't support complex regex. And I use scanf to make testing this code simpler. Later, I replace it with another function that reads user input. Test is an array test[40] that is declared in main.

In addition there is a compiler warning about my for loop, that value computed is not used. It probably means *ch. I use it to end for loop when *ch reaches '\0'. I know I could use while loop. However, is there a solution to fix that or a workaround or a better solution using for loop? And I'd like to know, what would you do differently or what would you also check in addition to what I am checking in my code.

int test(char test[])
{
    int     i;
    char    *pos1, *pos2;
    char    *ch;

    puts("Enter email!");
    scanf("%39s", test);

    while (1) {
        for (i = 0, ch = test; *ch; *ch++) {
                if (*ch == '@') {
                    pos1 = ch;  // <-stores last @ address
                    i++;
                }
            }
            pos2 = ch;          // <-stores end address ("\0") of test

        /* If there is only one "@" */
        if (i == 1) {

            /* Pos1 - test <-a character must be before "@". Ch - pos1 <-Counts
             * backwards from the end of the string towards "@". A character
             * must be between "@" and "." */
            while (pos1 - test && ch - pos1 > 1) {

                /* From the end of string to "." has to be atleast 2 chars */
                if ((pos2 - ch) > 2 && *ch == '.') {
                    printf("pos2 - ch is %d and *ch is %c",pos2 - ch, *ch);
                    return 1;
                }
                ch--;
            }
        }
        puts("Email wrong! Enter again");
        scanf("%39s", test);
    } /* End while */

    return 1;
}

Formatting

for (i = 0, ch = test; *ch; *ch++) {
        if (*ch == '@') {
            pos1 = ch;  // <-stores last @ address
            i++;
        }
    }
    pos2 = ch;

You have an extraneous level of indentation in there that throws out that block.

int     i;
char    *pos1, *pos2;
char    *ch;

This is a weird indentation level, it doesn't match anything else and is unnecessary. It literally just complicates how you read it.

Naming

int test(char test[]):

Naming both your function and variables test is bad practice. How am I meant to tell as an outsider reading your code what it is that your code does.

/* Pos1 - test <-a character must be before "@". Ch - pos1 <-Counts
 * backwards from the end of the string towards "@". A character
 * must be between "@" and "." */

If you feel the need to explain your code with comment blocks like this, you can probably improve your naming.

Name your variables as to what they are. For Example: charactersAfterAt.

Duplicate Logic

scanf("%39s", test);
while(1){
    // ...
    scanf("%39s", test);
}

You can reduce duplicate logic by declaring the scan inside the loop at the top.


Procedural function for prepared MySQL statements in PHP I'm refactoring procedural code, switching to prepared MySQL statements using mysqli_query(). I do understand writing OO would be better, but it's out of scope for this task.

Use the below function as follows:

list($result, $numrows, $err) = my_mysqli_stmt($db_handler, $query, $params);

Returns array with three values, $result, $numrows and $err.

$result is either false on error, or contains empty or multi-dimensional array.

$numrows returns number of affacted rows, number of returned rows or 0 on error.

$err returns mysqli_error() or mysqli_stmt_error() error.

function my_dbstmt($handler, $query, $params=array())
    {
    $stmt = mysqli_prepare($handler, $query);

    if($stmt===false)
        {
        return array(false, 0, mysqli_error($handler));
        }

    //First two parameter of mysqli_stmt_bind_param
    $refarg = array($stmt, array2typestring($params));

    //create array of parameters' references
    foreach ($params as $key => $value)
        {
        $refarg[] = &$params[$key];
        }

    if(!empty($params))
        {
        //bind parameters with dynamic length
        call_user_func_array("mysqli_stmt_bind_param", $refarg);
        }

    $exec = mysqli_stmt_execute($stmt);

    if($exec===false)
        {
        return array(false, 0, mysqli_stmt_error($stmt));
        }

    $result = mysqli_stmt_get_result($stmt);

    if($result===false)
        {
        //not a result, but successfully executed
        $numrows = mysqli_stmt_affected_rows($stmt);
        return array(true, $numrows, null);
        }
    else
        {
        mysqli_stmt_store_result($stmt);

        $data = array();

        while ($row = mysqli_fetch_array($result, MYSQLI_ASSOC))
            {
            $data[] = $row;
            }

        $numrows = count($data);
        return array($data, $numrows, null);
        }
    }

For completeness (detail of above referenced function:

function array2typestring($array)
    {
    $types = array();

    foreach ($array as $key => $value)
        {
        $types[] = 's';
        }

    $types = implode('', $types);
    return $types;
    }

Example usage:

$db_handler = mysqli_connect("p:$dblocal", $dbuser, $dbpass) or die();
$query = "SELECT title FROM books WHERE id=?";
$params = array($bookid);
list($result, $numrows, $err) = my_dbstmt($db_handler, $query, $params);

Result:

$result holds multi-dimensional array with books' title (just one in this case). $numrows is 1. $err is null.

Catching errors would work as follows:

if($result===false) {print("Error finding book")}
elseif($numrows==0) {print("Book not found")}
elseif($numrows>0) {print("Book found")}

The main thing that needs to be refactored here is the approach itself. Calling a function like this

list($result, $numrows, $err) = my_mysqli_stmt($db_handler, $query, $params);

is absolutely not the way to go.

Definitely, there should be only one result, especially because this function returns superfluous variables which you don't really need.

$numrows is really useless as any time you need the count, you can count $result itself.

Returning $err is wrong too. You need to understand how a web-site works. If you take a look an any site around, including this one, none of them tells you silly errors like "Error finding a book". If an error occurs, the site just shuts down completely, because a database error is a disaster, and nothing useful will be shown anyway. that's why in case of error only a generalized page with excuses is shown. So in the end, in case of error, your function should return nothing, but actually throw an error to tell PHP that something terrible happened.

Therefore, the proper usage of such a function would be

$query = "SELECT title FROM books WHERE id=?";
$result = my_dbstmt($db_handler, $query, [$bookid]);

if(!$result) print("Book not found");
else print("Book found");

This is the most important thing about your function, while everything else is just nitpicks in comparison:

There is a built-in function str_repeat which is doing exactly the same as your array2typestring.
There is an argument unpacking operator which is doing exactly the same as your referencing magic
there is a mysqli_fetch_all() function which is...  so you get the idea.

using all this knowledge, you can write your function like this

function my_dbstmt($handler, $query, $params = [])
{
    $stmt = mysqli_prepare($handler, $query);
    $types = str_repeat('s', count($params));
    mysqli_stmt_bind_param($stmt, $types, ...$params);
    mysqli_stmt_execute($stmt);
    $result = mysqli_stmt_get_result($stmt);
    return mysqli_fetch_all($result);
}

The function above supports only SELECT queries. To make it support other queries is not a good idea, and better be implemented using different methods within the same class - the thing OOP is good for. But as you decided to stick with procedural, it could be done with different result type returned by this function: for SELECT queries it will return an array and for everything else the number of affected rows.

function my_dbstmt($handler, $query, $params = [])
{
    if ($params)
    {
        $stmt = mysqli_prepare($handler, $query);
        $types = str_repeat('s', count($params));
        mysqli_stmt_bind_param($stmt, $types, ...$params);
        mysqli_stmt_execute($stmt);
        $result = mysqli_stmt_get_result($stmt);
    } else {
        $result = mysqli_query($handler, $query);
    }
    if ($result)
    {
        return mysqli_fetch_all($result);
    } else {
        return mysqli_affected_rows($result);
    }
}

Now this function can be used with DML queries as well and even with any other type of query, like CREATE TABLE or SHOW PROCESSLIST.

Now you can ask what about errors. It should be no concern of this function at all. All database errors should be converted to PHP errors and treated exactly the same way. To convert mysqli errors to PHP errors you need only one command - mysqli_report(). So in the end your code should look like

mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
$db_handler = mysqli_connect($dblocal, $dbuser, $dbpass, $dbname)

$query = "SELECT title FROM books WHERE id=?";
$result = my_dbstmt($db_handler, $query, [$bookid]);

if(!$result) echo "Book not found";
echo print $result[0]['title'];

and for the DML query it would be

$query = "DELETE FROM books WHERE id=?";
$result = my_dbstmt($db_handler, $query, [$bookid]);
echo "$result books has been deleted";

Note that you'd better avoid blunt use of persistent connections as it will give you nothing you imagine but only problems.


Add 1 to a number represented as an array of digits, using recursion Given a non-negative number represented as an array of digits, plus one to the number.
  The digits are stored such that the most significant digit is at the head of the list.

Explanation for using array vs ArrayList:

I could have used ArrayList and that would have been simpler while appending a 1 at the beginning but I wanted to use arrays only as the functions declarations were given and I didn't want to keep converting list to array and vice versa there by adding another O(n) operation.

Any feedback is appreciated. I know there are similar questions in other languages and using iterative approach, but I want feedback on my particular approach.

public class plusOne {

    public static void main(String args[]){
        plusOne obj = new plusOne();
        int[] digits = {0};
        int[] result = obj.plusOne(digits);
        System.out.println(Arrays.toString(result));
    }

    public int[] plusOne(int[] digits) {
        int length = digits.length-1;
        return increment(digits, length);
    }

    public int[] increment(int [] digits, int index) {
        if(digits[index]!=9)
        {
            digits[index]+=1;
            return digits;
        } else {
            if(index==0) {
                int[] newDigits = Arrays.copyOf(digits,digits.length+1);
                newDigits[digits.length] = 0;
                newDigits[index]=1;
                return newDigits;
            }
            digits[index] = 0;
            return increment(digits, index-1);
        }
    }
}

First of all, please be consistent: int[] digits is the norm, but you also wrote String args[] and int [] digits.  Classes are normally named with an initial uppercase (PlusOne).  It is also customary to put a space before and after binary operators and after each comma, as well as after the if keyword.

There isn't any point in instantiating a plusOne object, when all you want is a function.  Therefore, you should have public static int[] plusOne(int[] digits).  The increment function is a helper; it should be private static int[] increment(…).

It's weird that the operation usually operates on the array in place, but sometimes writes "junk" to that array then returns a new one.  Perhaps that is a consequence of the nature of the problem, but it's an issue that should be clearly documented in JavaDoc.

In Java, it is usually preferable to avoid recursion (due to concerns about stack overflow and performance) if the problem can also be easily solved using iteration — as this problem can.  If you do use recursion, though, I recommend structuring the code to make it more obvious that there are two base cases and one recursive case.  The way the index == 0 case sets newDigits[digits.length] = 0 feels weird: it doesn't work locally the way recursion should.  If you're going to make the assumption that it's OK to plop a 0 at the end, you might as well assume that you want a 1 followed by 0s everywhere else, in which case you could just do new int[digits.length + 1].

public class PlusOne {

    private PlusOne() {}  // Suppress the default constructor

    /**
     * Adds one to an integer that is represented as an array of digits
     * (most-significant digit first).
     *
     * @return The returned array may or may not be the same array
     *         as the input.  In either case, the contents of the
     *         input array will be overwritten.
     */
    public static int[] plusOne(int[] digits) {
        return increment(digits, digits.length - 1);
    }

    private static int[] increment(int[] digits, int index) {
        if (digits[index] != 9) {
            digits[index] += 1;
            return digits;
        } else {
            digits[index] = 0;
            if (index > 0) {
                return increment(digits, index - 1);
            } else {
                int[] newDigits = new int[digits.length + 1];
                newDigits[0] = 1;
                return newDigits;
            }
        }
    }
}


Updated bash/zsh function that cd to the root of git tree here's the updated version of the code for being reviewed. Fixes:

"private" _cg function
functions definition as cg() {}
general clean-up

#!/bin/bash

# cd to the root of the current git directory
# If $PWD is submodule, will cd to the root of the top ancestor
# It requires to stay in the current directory, if the root is . or unknown,
# and use cd only once, to have a way to do `cd -`
cg() {
  _cg() {
    local top; top="$(git rev-parse --show-cdup)"
    top="${top:-./}"
    local super_root; super_root="$(git rev-parse --show-superproject-working-tree)"
    if [[ "$super_root" ]]; then
      printf '%s' "$top../"
      ( cd "$top../" && _cg || return )
    fi
    printf '%s' "$top"
  }
  local git_root
  git_root="$(_cg)"
  [[ "x${git_root}" != "x./" ]] && cd "${git_root}" && return || return 0
}

This is looking great.  Thanks for incorporating the previous improvements and coming back for another round of review.  I noticed a few new things this time:

What does printf '%s' do for you that echo -n wouldn't?  Using printf when you're just passing through a string seems a little odd to me.  It should work either way, but printf would be more idiomatic for C where echo is much more common in shell scripts.
Adding the x constants in the comparison [[ "x${git_root}" != "x./" ]] are not needed.  Using the double square brackets deals with the disappearing variables.  You would need the extra x for [ comparisons because empty arguments become non-existant arguments which causes an error when you try to compare the non-existant to something.
I would use -n in the if [[ "$super_root" ]]; then line like so: if [[ -n "$super_root" ]]; then to make clear what you're trying to do there.


Java Generics and ArrayLists This abomination of code creates arrays based off input from the user. Sample input would be something like "S 4 aba bab baa aab" where:

args[0] = array type (Int, Double, String)
args[1] = length of array
args[2...] = contents of the array

The creation of the arrays in my code is horrible, but I am not quite sure how else to do it while incorporating generics. The point of the code is to be able to pass any of the created arrays to the methods at the bottom, hence generics.

import java.io.FileNotFoundException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;

public class Prog7a<E> {
public static void main(String[] args) throws FileNotFoundException {

    checkUsage(args);

    //This would be nice, but I get "Cannot make static reference to non-static E"
    //      ArrayList<E> array = new ArrayList<>();
    //
    //      if (args[0].matches("I|i"))
    //          for (int i = 2; i < args.length; i++)
    //              array.add((E) new Integer(args[i]));
    //
    //       else if (args[0].matches("S|s"))
    //           for (int i = 2; i < args.length; i++)
    //                  array.add((E) new String(args[i]));
    //       else
    //           for (int i = 2; i < args.length; i++)
    //                  array.add((E) new Double(args[i]));
    //
    //
    //      System.out.println("Original: " + array);
    //      System.out.println("  Unique: " + removeDuplicates(array));
    //      shuffle(array);
    //      System.out.println("Shuffled: " + array);
    //      System.out.println(" Maximum: " + max(array));

My question is here: I have the loop to fill in the array and call to the methods in each conditional, which makes me think I could rewrite this in such a way that it is not so repetitive.

    if (args[0].matches("I|i")) {
        ArrayList<Integer> iL = new ArrayList<Integer>();
        for (int i = 2; i < args.length; i++) {
            iL.add(Integer.parseInt(args[i]));
        }
        System.out.println("Original: " + iL);
        System.out.println("  Unique: " + removeDuplicates(iL));
        shuffle(iL);
        System.out.println("Shuffled: " + iL);
        System.out.println(" Maximum: " + max(iL));

    } else if (args[0].matches("S|s")) {
        ArrayList<String> sL = new ArrayList<String>();
        for (int i = 2; i < args.length; i++) {
            sL.add(args[i]);
        }
        System.out.println("Original: " + sL);
        System.out.println("  Unique: " + removeDuplicates(sL));
        shuffle(sL);
        System.out.println("Shuffled: " + sL);
        System.out.println(" Maximum: " + max(sL));
    } else {
        ArrayList<Double> dL = new ArrayList<Double>();
        for (int i = 2; i < args.length; i++) {
            dL.add(Double.parseDouble(args[i]));
        }
        System.out.println("Original: " + dL);
        System.out.println("  Unique: " + removeDuplicates(dL));
        shuffle(dL);
        System.out.println("Shuffled: " + dL);
        System.out.println(" Maximum: " + max(dL));
    }

}

public static <E> ArrayList<E> removeDuplicates(ArrayList<E> list) {
    new HashSet<E>(list);
    list = new ArrayList<E>(new HashSet<E>(list));
    return list;

}

public static <E> void shuffle(ArrayList<E> list) {
    Collections.shuffle(list);
}

public static <E extends Comparable<E>> E max(ArrayList<E> list) {
    return Collections.max(list);
}

public static void checkUsage(String[] args)
{
    if (args.length < 2)
    {
        System.out.println("Please supply more than 2 arguments");
        System.exit(1);
    }
    if (!(args[0]=="s") && (args[0]=="S") &&
        (args[0]=="d") && (args[0]=="D") &&
        (args[0]=="i") && (args[0]=="I")){
        System.out.println("Incorrect format of command.");
        System.exit(1);
    }
}
}

There are several problems with your code. I will start with the method checkUsage(String[]). The check for the number of arguments should be if (args.length <= 2), with a <= instead of a < operator (or, alternatively, if (args.length < 3). Then, the check of the arguments themselves is faulty, because the condition you specify for an incorrect format will never evaluate to true. To check if none of two conditions hold true, you must either test

if (!(condition1 || condition2))

or

if (!condition1 && !condition2).

The two are equivalent, but be aware of operator precedence: The unary operator ! will be evaluated before || or &&, so in the first case, you have to surround the expression condition1 || condition2 with parentheses.

Next, you are comparing Strings using ==. This is dangerous, because == only checks for reference equality but not the contents of the Strings. You should rather use String.equals(Object) to compare Strings. In your case, you could even take advantage of String.equalsIgnoreCase(String), halving the number of method invocations and making the code easier to read.

So much for the method checkUsage(String[]). On to the methods shuffle(ArrayList) and max(ArrayList). I don't really see a point in defining these methods, since they do nothing but pass the argument to the respective method in Collections, so you might as well call Collections.max(Collection) and Collections.shuffle(List) directly from main(String[]).

The method removeDuplicates(ArrayList) unnecessarily creates a HashSet in the first line that will never be used, because the expression new HashSet<E>(list) passed to the ArrayList constructor in the second line creates a new HashSet that has nothing to do with the HashSet created in the first line, so the first line is completely redundant. Also be aware that HashSet.iterator() returns the elements contained in the Set in no particular order, so your method removeDuplicates(ArrayList) might not only remove duplicates, but also change the order of the List in general. Furthermore, you might want to consider defining the method parameter as a List (which is an interface) instead of an ArrayList (which is a class), because the method doesn't depend on the specific implementation of the passed List but only on its behavior, which is defined by the interface. That way, the method can also accept other implementations of the List interface and not only ArrayLists. And lastly, the name "removeDuplicates" is a bit misleading, since, unlike shuffle(List), the method doesn't actually modify the passed List but returns a new List without duplicates, leaving the passed List unmodified. Maybe "withoutDuplicates" would be better, or something like that.

Now, about the generic type parameter E. There is no point in defining it in the class declaration because the class itself does not depend on it, only your methods that accept an ArrayList (or List) as a parameter do. I suggest you remove it, because defining the class Prog7a with a generic type parameter <E> and then declaring methods that define a generic type parameter <E> again introduces ambiguity:

public class Prog7a<E> { /* If we instantiate a Prog7a<String>,
                            this E would be String */

    /* Now comes a method declaration that defines a generic
    type parameter <E> again. This newly defined parameter
    hides the parameter defined in the class declaration and is
    completely independent of it, so this method might
    very well accept a List<Integer>, even though the type
    of this class is Prog7a<String> */
    public <E> ArrayList<E> removeDuplicates(List<E> list) {
        // ...
    }
}

Regarding the actual problem at hand, which is the code repetition in your original code, the other two suggested solutions both seem appropriate to me, but I don't get the organization of slowy's code at all. Not only that the execution logic of the program is put inside a constructor, but the constructor actually doesn't serve any other purpose at all except establishing a type parameter, and the generic type parameter of the Prog7a class is unnecessary, as I already explained. This also means that your static methods can, indeed, remain static.


Java 8 use of optional I have two methods I'd like to improve using optional and any other tool,

 @Override
    public UserDetails loadUserByUsername(String username) throws UsernameNotFoundException {
        User user = userRepository.findByUsername(username);
        if (user == null) {
            throw new UsernameNotFoundException(String.format("No user found with username '%s'.", username));
        } else {
            return user;
        }
    }

    public void changePassword(String oldPassword, String newPassword) {

        Authentication currentUser = SecurityContextHolder.getContext().getAuthentication();
        String username = currentUser.getName();

        if (authenticationManager != null) {
            LOGGER.debug("Re-authenticating user '"+ username + "' for password change request.");

            authenticationManager.authenticate(new UsernamePasswordAuthenticationToken(username, oldPassword));
        } else {
            LOGGER.debug("No authentication manager set. Can't change Password!");

            return;
        }

        LOGGER.debug("Changing password for user '"+ username + "'");

        User user = (User) loadUserByUsername(username);
        user.setPassword(passwordEncoder.encode(newPassword));
        userRepository.save(user);

    }

First solution was to do
Optional.ofNullable(user).orElseThrow() but I couldn't use the string format here for the UsernameNotFoundException. I don't want to lose the message I have written in the exceptions.

Any suggestions on how to improve this code?

Optionals do not improve your code. You're using if statements and null checks just like they are intended to be used. There is no need to improve anything there.

Edit: If-statements and null-checks are not evil. There is no need to try to get rid of either one and Optionals were not intended to replace them. The evil is having an API made by someone else and not knowing if it returns a null or not. Optionals were intended to give the person reading the code and the compiler compiling it a strongly typed fact about the possibility of a method return value not existing.

If I have to try to find something worth mentioning from your code it's the use of heavy String.format to create a trivial exception message. Just use concatenation like in the logging statements. :)


Parse a list of people to determine who likes someone I have a data file that looks like this:

Friend Name                     Joe
Likes                           Jill
Age                             30
Gender                          Male

Friend Name
Likes                           Mark
Age                             30
Gender                          Male

Friend Name                     Bob
Likes                           Ralph
Age                             30
Gender                          Male

Note: the data file has leading spaces before the first column.

I want to parse this file and see who likes a given person (likes). In this example, at most only one person likes the given person. So, say I am finding who likes 'Ralph', only Bob does. It is possible that the likes is not even in the data. It is also possible that the Friend Name might be blank (see the second Friend).

Here is my code to parse this data and return the friend who likes 'likes':

def parse_friends_list(friends_list, likes):
    output = friends_list.strip().splitlines()
    for line in output:
        try:
            if line.startswith('Friend Name'):
                friend = line.split()[2]
            elif line.startswith('Likes'):
                found_likes = line.split()[1]
                if found_likes == likes and friend:
                    return friend
                friend = ''
        except IndexError:
            pass
    raise NoFriendParse('Unable to find a friend for {}'.format(likes))

I like to break problems like this up into a few steps, makes it easier for my brain to understand.

import re

raw_text = """Friend Name                     Joe
Likes                           Jill
Age                             30
Gender                          Male

Friend Name
Likes                           Mark
Age                             30
Gender                          Male

Friend Name                     Bob
Likes                           Ralph
Age                             30
Gender                          Male"""

def split_row(row):
    items = re.split('\s\s+', row)
    if len(items) < 2:  # Value is blank
        return items[0], None
    return items

def parse_block(block):
    rows = block.split('\n')
    fields = map(split_row, rows)
    return dict(fields)

def process_text(text):
    blocks = text.split('\n\n')
    return map(parse_block, blocks)

if __name__ == '__main__':
    for like in process_text(raw_text):
        print(like)

Which generates the output

{'Likes': 'Jill', 'Friend Name': 'Joe', 'Age': '30', 'Gender': 'Male'}
{'Likes': 'Mark', 'Friend Name': None, 'Age': '30', 'Gender': 'Male'}
{'Likes': 'Ralph', 'Friend Name': 'Bob', 'Age': '30', 'Gender': 'Male'}


Count and group rows for six months prior I've written the following query for SQL server:

SELECT YEAR(Meeting_Date) year, MONTH(Meeting_Date) month, COUNT(project) meetings
FROM
meetings
WHERE Meeting_Date BETWEEN
CAST(DATEPART(year,DATEADD(m,-6,GETDATE())) as varchar(4)) + '-' + CAST(DATEPART(month,DATEADD(m,-6,GETDATE())) as varchar(2)) + '-01'
AND
DATEADD(d,-1,CAST(DATEPART(year,GETDATE()) as varchar(4)) + '-' + CAST(DATEPART(month, GETDATE()) as varchar(2)) + '-01')
GROUP BY YEAR(Meeting_Date), MONTH(Meeting_Date)
ORDER BY YEAR(Meeting_Date), MONTH(Meeting_Date)

This code counts up how many meetings there were for the six months prior to this one.  It works perfectly, but I'm sure there's a better way of figuring the months than I have.

This is one of those things.... date manipulation in SQL.

For performance reasons, it is a pain to cast to NVarchar, etc. For readability reasons, everything is a pain.....

What I have found works best for me, is the following function:

DateAdd(d, 1 - DatePart(d, actualdate), actualdate)

The above subtracts the number of days needed to bring a date back to the first-of-the-month.

For example today, (the 28th April), would subtract 1 - 28 (or 27) days from the 28th, to get the 1st.

You can do your current month-manipulation to move months around once you are back on to the first-of-the-month, and then you don't have to worry about the month overflow problems..

For readability reasons, I would recommend you use variables, if you can:

declare @thismonth as Date = Convert(DateAdd(d, 1 - DatePart(d, CURRENT_TIMESTAMP), CURRENT_TIMESTAMP) as Date);
declare @firstmonth as Date = DateAdd(m, -6, @thismonth);

SELECT YEAR(Meeting_Date) year,
       MONTH(Meeting_Date) month,
       COUNT(project) meetings
FROM
    meetings
WHERE Meeting_Date BETWEEN @firstmonth AND @thismonth
GROUP BY YEAR(Meeting_Date), MONTH(Meeting_Date)
ORDER BY YEAR(Meeting_Date), MONTH(Meeting_Date)


PHP MySQLi database object I've been learning more about PHP Objects and Classes, and my immediate reaction was to create a Database Object for handling the MySQLi Connection, Statements and Results.  Its not meant to be an ultimate do-all object, but help with WET code.  Obviously this has been done before... at least a hundred times.

There's no question lingering on whether I should be doing this, my question is more along the lines of am I doing this right?.

Basically I don't want to write out 10+ lines of code each time I make an SQL statement, let alone deal with the other functions that take place.  So I made an object that can do the SQL connection, statement, and get results.  The goal was to make the interaction as simple as possible and I like to think I achieved that.  Thoughts?

Usage:

Create the Object

$db = new Sqli();

Execute a Statement

$db->statement($sql, $param)

$db->statement("SELECT column_name FROM table WHERE = ?", "bind_me")

$db->statement("INSERT INTO table (col1, col2, col3) VALUES (?, ?, ?)", [$foo, $bar, $baz]);

Print Results

print_r($db->result());

JSON Result

print json_encode($db->result());

PHP Code:

class Sqli
{
    const DBHOST = "localhost";
    const DBUSER = "";
    const DBPASS = "";
    const DBNAME = "";

    protected $conn;
    protected $stmt;

    function __construct()
    {
        $this->setConnection();
    }

    private function setConnection()
    {
        mysqli_report(MYSQLI_REPORT_STRICT|MYSQLI_REPORT_ERROR);
        try
        {
            $conn = new mysqli(
                self::DBHOST,
                self::DBUSER,
                self::DBPASS,
                self::DBNAME
            );
        }
        catch(MySQLi_sql_exception $e)
        {
            throw new \MySQLi_sql_exception(
                $e->getMessage(),
                $e->getCode()
            );
        }
        $this->conn = $conn;
    }

    public function statement($sql, $param)
    {
        $stmt   = $this->conn->prepare($sql);

        if($param !== FALSE)
        {

            if(!is_array($param))
            {
                $param = [$param];
            }

            $types = str_repeat("s", count($param));

            $stmt->bind_param($types, ...$param);
        }

        $stmt->execute();

        $stmt->store_result();

        $this->stmt = $stmt;
    }

    public function result()
    {
        $stmt = $this->stmt;

        $meta = $stmt->result_metadata();

        while($field = $meta->fetch_field())
        {
            $param[] = &$row[$field->name];
        }

        call_user_func_array([$stmt, "bind_result"], $param);

        while($stmt->fetch())
        {

            foreach($row as $key => $val)
            {
                $r[$key] = filter_var($val, FILTER_SANITIZE_FULL_SPECIAL_CHARS, FILTER_FLAG_ENCODE_HIGH|FILTER_FLAG_ENCODE_LOW|FILTER_FLAG_ENCODE_AMP);
            }
            $result[] = $r;
        }

        return $result;
    }
}

Dont call private methods in constructor, put the code into the constructor directly (well unless you have reason to call the same piece of code from elsewhere too, not your case tho).

Don't put your db credentials into the class's constants. This way you are unable to use the class to connect to any other database. Well you can change their values, but that means change the code, or you can extend the class and override the constants. But that's also not very convenient.

You might want to prefer PDO instead of the mysqli extension. The PDO contains mysql adapter in it (maybe you need pdo_mysql extension i think). But you will have the freedom to change the underlying database transparently anytime (well unless you are using some specific sql dialect features).

PDO basically offers the same as your class does, except it has no hardcoded credentials, which, as I already mentioned, is bad.

So learn to use PDO and you may find out that you need no such wrapper at all, you just need a place where you pass the right credentials to its contructor. This place could be described as database connection factory.


Get the first 10 lines of a file Print the first 10 lines of a file, if it has less than 10 lines, print the whole file.

Any suggestions on any kind of improvement are welcome.

package je3.io;

import java.io.BufferedReader;
import java.io.FileReader;
import java.io.IOException;

/**
 * Created by IDEA on 29/01/15.
 */
public class Head {
    public static String[] headLines(String filename) throws IOException {
        BufferedReader reader = new BufferedReader(new FileReader(filename));
        String[] lines = new String[10];
        String line;
        int nLine = 0;
        while((line = reader.readLine()) != null) {
            lines[nLine++] = line;
            if(nLine >= 10) {
                break;
            }
        }
        return lines;
    }

    public static void main(String[] args) throws IOException {
        if(args.length != 1) {
            throw new IllegalArgumentException();
        }
        String[] lines = headLines(args[0]);
        for(String line : lines) {
            if(!line.isEmpty()) {
                System.out.println(line);
            }
        }
    }
}

This code is just begging for a Java8 implementation.... and, given that Java 8 has been available for almost a year, it seems reasonable.

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Paths;

public class Head {

    public static void main(String[] args) throws IOException {
        if (args.length != 1) {
            throw new IllegalArgumentException();
        }

        Files.lines(Paths.get(args[0])).limit(10).forEach(System.out::println);
    }

}


Constructing NewsArticle objects from a database table This class seems to be able to represent the data stored in any possible database table:

public class DbTable {
    private Set<Map<String, Object>> tblData = new HashSet<Map<String, Object>>();

    public DbTable(Connection dbConn, String tblName) throws Exception {
        PreparedStatement prpStmt = dbConn.prepareStatement("SELECT * FROM " + tblName);
        ResultSet queryResults = prpStmt.executeQuery();
        ResultSetMetaData metaData = queryResults.getMetaData();

        while (queryResults.next()) {
            Map<String, Object> row = new HashMap<String, Object>();

            for (int i = 1; i <= metaData.getColumnCount(); i++) {
                String colName = metaData.getColumnName(i);
                String colClassName = metaData.getColumnClassName(i);
                Class colClass = Class.forName(colClassName);
                Object colValue = colClass.cast(queryResults.getObject(i));

                row.put(colName, colValue);
            }
            tblData.add(row);
        }
    }

    // ...
  }

Then, I'd like to construct objects to represent news articles based on the Map<String, Object> retrieved via DbTable, like this:

public class NewsArticle {
    private final String Id, Title, Story;
    private final Timestamp Pubdate;

    public NewsArticle(Map<String, Object> row) {
        String id = null; String title = null; String story = null;
        Timestamp pubdate = null;

        for (Map.Entry<String, Object> col : row.entrySet()) {
            if (col.getKey().equals("id")) {
                id = (String) col.getValue();
                continue;
            }
            if (col.getKey().equals("pubdate")) {
                pubdate = (Timestamp) col.getValue();
                continue;
            }
            if (col.getKey().equals("title")) {
                title = (String) col.getValue();
                continue;
            }
            if (col.getKey().equals("story")) {
                story = (String) col.getValue();
            }
        }

        Id = id;
        Pubdate = pubdate;
        Title = title;
        Story = story;
    }

    // ...
}

So, a Map represents a database table entry as the column name, mapEntry.getKey(), and the data in the column, mapEntry.getValue().

Because I lose type safety, I'm thinking this is rubbish. But, (1) users of DbTable most certainly should know the type of each column. (2) if they do not know the type, then they can use reflection as a backup way to make DbTable usable.

This solution seems to work fine. Any opinions about losing that type safety? How could I improve this?

Coding Issues

In NewsArticle, the names of the attributes begin with uppercase letters (Id, Title, Story, Pubdate). This violates common Java field naming conventions.
Database column names are usually case insensitive. Depending on the underlying DBMS, the instructions like col.getKey().equals("id") may fail, because the searched value could have been stored under e.g. ID key when being read in DbTable. Either it should be normalized by applying toLowerCase() there, or it should be checked with equalsIgnoreCase() in NewsArticle constructor.
Looping over the entire row map in NewsArticle constructor is unnecessary. The fields may be initialized with e.g. id = (String) row.get("id"); and there is no need to use local variables id, title etc.
In DbTable: Class colClass var is not typed (raw type), which is not good. This variable is useless, because colValue is an Object and can be directly initialized with queryResults.getObject(arg), without casting.

Design Issues: Critical!

There are some very important issues concerning DbTable class.

Loading the contents of an entire table using tblData Set is very dangerous. First of all, if the table is rather huge (imagine just several dozens of thousands of rows), this object will squatter the memory and probably consume too much of it uselessly. It also duplicates the data supposed to be stored in the DB and produces too much overhead on each DbTable object creation by reading all the data. However, reading just the metadata of table columns and their types could have been useful for your problem.
prpStmt and queryResults objects are created, but never closed. Initializing them inside a try-with-resources block will solve the issue.
throws Exception, especially in a constructor, looks very ugly. Either replace it with a more concrete exception type or (even better) find a way to avoid the throws clause for the constructor.
There are no validity checks for Connection and String arguments. What will happen, for example, if one of them or both are null?

These remarks do not answer your initial question about the type safety of the solution. It's too early to answer it before fixing many of the critical issues.