• Prime Mover (unregistered)

    Actually no:

        get
    {

    is wrong, because it should be:#

        get {
  • dpm (unregistered)

    Maybe my ignorance is showing, but I can't figure out why you state (twice) that only ten results are coming back in the assignment to "pages". I've googled Episerver's GetContentResult() method and found no mention of any default limit, and there's certainly nothing in this horrible code that limits that result, so what am I missing?

  • (author) in reply to dpm

    In this case, that detail was provided by the submitter- I assume there's a configuration somewhere in their environment.

  • 516052 (unregistered) in reply to Prime Mover

    No. Bad programmer. The only proper way to start a scope is by placing the curly brace on a NEW LINE. What you are writing is Java.

  • RLB (unregistered) in reply to 516052

    The only proper way to start a scope is by placing the curly brace on a NEW LINE.

    Kernighan and Ritchie would like a word.

  • Tom (unregistered)

    Another WTF is having a property that fetches data from the network. A property should be fast, and it's not the case here. It should be a method, preferably asynchronous

  • dpm (unregistered) in reply to 516052

    public int YourStatementIsNotAbsolute { get; set; }

  • dpm (unregistered) in reply to Remy Porter

    I can't figure out why you state (twice) that only ten results are coming back

    that detail was provided by the submitter

    Pretty damned important detail --- that code reads as if most of the table is being returned

  • (nodebb)

    Hold on, if the requirement is get most recent 5 published articles, why do you need elastic search at all? Why not just query the data source directly? Like SELECT TOP 5 Title, Contents, Date FROM Article WHERE NewsType = 'bla' AND Published = 1 ORDER BY Date DESC; if it's a SQL database.

  • (nodebb)

    I love the "JsonIgnore" attribute. At some point, they tried serializing an instance of the class which contains this property, (WHY??!!!!) and serialization either took a long time or failed altogether, and somebody must have debugged it and realized, oh wait, this property is doing a data retrieval operation.

  • anonymous (unregistered) in reply to 516052

    Some people get paid by lines of code, some get paid by "done well".

  • (nodebb) in reply to anonymous

    It's not about how many lines you write, it's the readability. I always find it extremely annoying to read code with opening curly brace on the same line because it's difficult to match it to the closing brace. Consistency is king.

    Addendum 2021-07-28 08:54: Also, methods have their opening brace on new line anyway; why introduce a different style to control blocks for the sake of making code more sense and harder to read? It's an all around bad practice.

    Addendum 2021-07-28 08:54: Dense not sense

  • 516052 (unregistered) in reply to Mr. TA

    Same here. The point is the make sure your visual scope matches your code. So if you have a very short statment like a { get; set; } or an IF that has a very short outcome like say IF( condition) { var = value } or something similar where it all fits in one line than yes, keep it in one line.

    {
   get;
   set;
}

    Is just stupid and visual clutter.

    But equally, if your code does have to break lines anyway than push that opening brace down as well so as to create a visual block that one can skim over or into easily with ones eyes without thinking about it.

  • Daniel Orner (github) in reply to RLB

    Also, the people who wrote Golang, because it is literally impossible to write code that starts brackets on new lines in that language.

  • Brian (unregistered) in reply to Tom

    Agreed, a property accessor that's more than a line or three is a serious code smell. If you must do all that work in an accessor, at least use a lazy-loading pattern:

     private blah _foo;
 public blah Foo => _foo ?? _foo = GetFoo();

    Bonus - you don't have to worry about where the brackets go :P

  • my name is missing (unregistered)

    The question I have is how did someone become a senior architect and write such poorly "architected" idiocy. Sadly most architects I have known never write code at all and this is the end result. I suppose someone has to the tail on the bell curve.

  • 516052 (unregistered) in reply to my name is missing

    How many architects do you know are skilled at bricklaying? Same story here.

  • I dunno LOL ¯\(°_o)/¯ (unregistered)

    Not knowing how this particular thing works, I certainly hope that the LINQ of the first search doesn't send an SQL query to get every article from the database, then remove things from those results. Because we know how code monkeys love to do that.

  • Anon (unregistered) in reply to Mr. TA

    Probably because Elasticsearch IS the data source.

    I hate it, but I have to admit it does make sense (having been in this situation before). If it's things like news articles, it makes sense to stuff them in Elasticsearch because it gives you easy access to things like fulltext search. Actually, it's probably 90% about the fulltext search ability. (It's not being used here but I would bet real money it's being used heavily elsewhere.)

    It's annoying, but it does make sense.

  • Yowzers (unregistered)

    Thanks for the crash course in C# / ElasticSearch.

    Possibly a WTF is serving out the latest 5 news articles of a news type (genre?), which seems to spit in the face of all the proven content recommendation algo's. Are you trying to cultivate news addicts here or just share the news??!

  • HermitDev (unregistered)

    Is no one even going to bother mentioning validating 'this.NewsType' after doing the query?

  • Officer Johnny Holzkopf (unregistered) in reply to 516052

    A common understanding is that if you encounter a closing }, you already know that it was caused by an opening {. You are not primarily interested in finding the matching {, but instead need to know what introduced that {, and therefore, you expect it at the same column as the closing }. The code that is depending on the construct is indented properly, so you can easily spot it, and also understand it as some kind of "encapsulated unit". This of course only works if you have consistent indentation, and it doesn't really matter if that is a fixed amount of spaces or actual tabs (which can be displayed as any amount of spaces the programmer desires, so that can be an advantage to change "indentation density", even in some of the simplest editors in case no real editor or IDE should be at hand). On the other hand, if you use a { on a separate line directly below the construct that caused it, you can achieve the same result just with one additional "visual vertical hop". Depending on the editor or IDE in use, selecting a } can quickly show you the corresponding { line, or lines. Languages like Python abandon the "block maker" { ... } altogether and require the programmer to use indentation for that purpose: defective indentation changes program structure, whereas the placement of { and } as well as the kind of indentation in C-like languages does not - overall it is just a cosmetic discussion.

  • Martin (unregistered) in reply to Mr. TA

    Writing an ES search query is hard as fuck, the dev probably gave up and said fuck it Linq will optimise it for me anyway.

  • 516052 (unregistered) in reply to Brian

    Personally I'd use a proper IF structure for that. Your version does not lend it self too well to debugging either via a debugger or via log statments. And yes, we do still use logs for debugging. that's just the price of working with proprietary hardware.

  • HG (unregistered) in reply to RLB

    Kernighan and Ritchie would like a word.

    Ah, but how long will that word be?

  • (nodebb)

    As long as 10 articles are displayed, it will be tough for the testers!

  • MaxiTB (unregistered) in reply to Mr. TA

    I think this wrote someone who actually has not experience in C# at all, because any more seasoned .net developer knows that properties should behave like fields and not like methods, so no unpredictable and long run times, no side effects (thanks god at least that one isn't there) and especially no dependency outside of the scope of the class;so holy, relying even on something that isn't located on the machine is a big no-go and beyond bad practice for a property.

    Secondly, it's elastic, so this code can't be that old. Why is it not async? It is also bad practice to use hardware resources synchronously except in few very specific cases. That point alone makes it clear that this code should never ever be in an property; because of the good practice rules above it is instantly clear that asynchronous properties make no sense :-)

  • MaxiTB (unregistered) in reply to Prime Mover

    This is incorrect.

    C# has actually a style guide since .net 1.0 which was only adjusted in minor ways when it comes to formatting and naming conventions over time (specifically private member fields ... originally they followed the convention m_<name>, these days some people prefer either this or simply _ as prefix, which is IMO the more readable option, even tho it doesn't align with the more Java focused clean code book recommendation).

    The correct c# bracket formatting is either: internal void LongMethod() { // Long method // }

    or

    private sealed override async Task<T> ShortMethodAsync<T>(CancellationToken cancellation = CancellationToken.None) { // Short statement // }

    But in the later case Lambda notation is preferable anyway.

    Brackets therefore are always on the same spacing level, no confusing mismatches and guessing which closing bracket belongs to which closing bracket, just clean visual scoping - which is for example completely different from C/C++ or Java which have a more chaotic approach with brackets following the previous scope line.

  • (nodebb) in reply to MaxiTB

    Apparently, either you're claiming that all braces and function bodies should be on the same line, or you're failing to write code fragments

    ```
like this.
```
  • (nodebb) in reply to dkf

    Apparently, either you're claiming that all braces and function bodies should be on the same line, or you're failing to write code fragments

    To be fair, this comment system is a minor WTF, because it gives absolutely zero indication of what syntax can be used. In the absence of any indication, assuming verbatim plain-text is reasonable. Especially, since there is no preview option (live or otherwise).

    Addendum 2021-07-31 15:49: Please add a "how to format" link :(

  • (nodebb)

    { placing

    Python: Hold my beer.

    # Just random code for demonstration of the formatting issue.
if (os.path.basename(infilename).startswith("userdata") and
    os.path.basename(outfilename).endswith(".xml")):
    assert False, "Please don't."

    I have yet to find any recommendation how to handle lengthier conditions, that does not result in making the code structure hard to see. For that matter, Python's "indentation as blocks" sometimes makes the code structure often unnecessarily hard to see to begin with, especially for code with complicated nesting structures.

    In that regard, curiously, unix scripts are nice.

    if [[ $(basename "$inputfilename") = "userdata"* ]] && \
   [[ $outputfilename = *".xml" ]]
then
    echo "Please don't."
    exit 1
fi

    Curiously, Fortran of all things almost has that in the language, but makes the worst of it, by requires the THEN to be part of the same line.

    ! Assumes some helper functions
IF( startswith("userdata", basename(inputfilename)) .AND. &
    endswith(".xml", outputfilename)) THEN
    ERROR STOP "Please don't"
END IF

    At which point I can do

    IF( startswith("userdata", basename(inputfilename)) .AND. &
    endswith(".xml", outputfilename)) &
THEN
    ERROR STOP "Please don't"
END IF

    but trying to trick the language is awkward, will likely improve the readability only for myself, and has a good chance of confusing some editors auto-indent feature.

    As for brace'd languages? Personally I prefer the terse

    if(inputfilename.startsWith("userdata")) {
    doIt();
}

    but in the case of block-head clauses with more than one line, I'd rather have

    if(baseinputfilename.startswith("userdata") &&
   outputfilename.endswith(".xml"))
{
    doIt();
}

    ... ... no, still confusing.

  • (nodebb) in reply to RLB

    Kernighan and Ritchie are wrong. Eric Allman would like a word.

Leave a comment on “All the News You Need”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article